Thread: C help
View Single Post
Old 05-09-07, 11:20 AM   #2
wnd
Nerd, Geek, Freak
 
wnd's Avatar
 
Join Date: Sep 2005
Location: Finland
Posts: 703
Default Re: C help

Quote:
Originally Posted by evilghost
Here is an example argv[1] passed value:
/www/dlf/dlf-albums/12-24 Months/Big Bubble bath 1.JPG

1) Check that argv[1] does not traverse outside of /var/www/dlf-albums
2) Use fprintf() instead of fputs() (isn't fputs less secure)?
3) For some reason asctime() is adding a newline character to the end of the string?
4) Be able to print output using something like fputs(fout,"%s\t%s\n",asctime(time(NULL)),argv[1]); without segfaulting
5) Create as secure code as possible.
1. If you only want to make sure only legimate paths pass through, you could simply make sure the string begins with your basepath, and no "/.." exists in the path. Maybe something in spirit of...
Code:
if (argc != 2 || argv[1] == NULL
                || strncmp(argv[1], BASEPATH, strlen(BASEPATH)) != 0
                || strstr(argv[1], "/..") != NULL) {
        return EXIT_FAILURE;
}
You will also have to make sure the target file is not a symlink. stat(2) and S_ISLNK(2) should help here.

2. Both will overflow if input string is not terminated. fsnprintf help a bit, but strings pointed by *argv are always terminated unless something is seriously broken. Always pass user-supplied string "through %s" (i.e. printf("%s", input); ), even when unnecessary. Sooner or later you forget to do it when it really counts.

3. The reason is described in the manual. :-) Use strftime if this bothers you.

4.
Code:
char t[80];
t[79] = '\0';
/* checking return value of strftime isn't necessary since t is guaranteed
   to be terminated */
strftime(t, 79, "%a %b %H:%M:%S %Y", loctime);
fprintf(fout, "%s\t%s\n", t, file);
5. Return values. Namely, check return value of fopen and avoid potential segfault.

Maybe you could consider writing the log with syslog(3).
__________________
web | cat

Christianity, noun: The belief that a cosmic Jewish Zombie who was his own father can make you live forever if you symbolically eat his flesh and telepathically tell him you accept him as your master, so he can remove an evil force from your soul that is present in humanity because a rib-woman was convinced by a talking snake to eat from a magical tree. [mad.frog]
wnd is offline   Reply With Quote