|
Vulnerability catdoc Affected Systems with catdoc-0.90 Description Duncan Simpson found following. catdoc-0.90 is full of buffer overruns. This could be a security problem if catdoc is used with privilege users do not have for automated indexing purposes or otherwise used with raised privilege. There are lots of overruns for bad guys to exploit. Kragen added this is not just a security problem if catdoc is run with "privileges users don't have" --- it's a security problem if you accept any documents from the outside world and then try to read them with catdoc, without first checking them to see if they have buffer-overflow attempts in them. Since, presumably, the usual reason one runs catdoc is that one person created a document with Microsoft products and another person, without Microsoft products available, tries to read that document, this is essentially a constant security hole. Solution The patch below fixes all of the bugs: --- src/catdoc.h~ Fri Sep 11 12:32:23 1998 +++ src/catdoc.h Thu Nov 5 03:28:36 1998 @@ -113,7 +113,7 @@ extern char* map_path, *charset_path; extern int signature_check; extern int unknown_as_hex; -void find_file(char *full_path,const char *name, const char *path); +void find_file(char *full_path,const char *name, const char *path, int size); void read_config_file(const char *filename); SUBSTMAP read_substmap(const char* filename); --- src/catdoc.c.dist Thu Nov 5 02:46:11 1998 +++ src/catdoc.c Thu Nov 5 04:13:43 1998 @@ -25,6 +25,20 @@ char **_argv; int _argc; #endif + + +/* Safe version of strcat(strcpy(<fixed size buf>, <...>), <...>) */ +static const char *safe_cpy_cat(char *buf, int bufsiz, const char *cp, const char *ad) +{ + int l; + + l=strlen(ad); + strncpy(buf, cp, bufsiz-l-1); + buf[bufsiz-l]='\0'; /* Ensure terminated */ + strcat(buf, ad); /* Safe because bufsiz-l is maximum start */ + return buf; +} + /**************************************************************/ /* Main program */ /* Processes options, reads charsets files and substitution */ @@ -42,7 +56,7 @@ #endif read_config_file(SYSTEMRC); #ifdef USERRC -find_file(tempname,USERRC,getenv("HOME")); +find_file(tempname,USERRC,getenv("HOME"), sizeof(tempname)); if (*tempname) { read_config_file(tempname); } @@ -97,9 +111,9 @@ target_charset= make_reverse_map(tmp_charset); free(tmp_charset); -spec_chars=read_substmap(strcat(strcpy(tempname,format_name),SPEC_EXT)); +spec_chars=read_substmap(safe_cpy_cat(tempname, sizeof(tempname), format_name, SPEC_EXT)); if (!spec_chars) exit(1); -replacements=read_substmap(strcat(strcpy(tempname,format_name),REPL_EXT)); +replacements=read_substmap(safe_cpy_cat(tempname, sizeof(tempname), format_name, REPL_EXT)); if (!replacements) exit(1); if (!isatty(fileno(stdout))) { --- src/charsets.c.dist Thu Nov 5 02:46:11 1998 +++ src/charsets.c Thu Nov 5 04:14:04 1998 @@ -60,11 +60,15 @@ short int *new=calloc(sizeof(short int),256); int c; long int uc; - strcpy (namebuf,filename); - strcat(namebuf,CHARSET_EXT); - find_file(filebuf,namebuf,charset_path); - if (!namebuf[0]) { + /* strcpy (namebuf,filename); + strcat(namebuf,CHARSET_EXT); */ + strncpy(namebuf, filename, sizeof(namebuf)-4-1); /* length of CHARSET_EXT is 4 */ + namebuf[sizeof(namebuf)-4]='\0'; /* Ensure null terminated */ + strcat(namebuf, CHARSET_EXT); /* Safe because enough space left by strncpy */ + + find_file(filebuf,namebuf,charset_path, sizeof(filebuf)); + if (!namebuf[0]) { fprintf(stderr,"Cannot load charset %s - file not found\n",namebuf); return NULL; } --- src/fileutil.c.dist Thu Nov 5 02:46:11 1998 +++ src/fileutil.c Thu Nov 5 03:30:08 1998 @@ -17,31 +17,56 @@ /* Searches for file name in specified list of directories. Sets */ /* full_path to empty string if nothing appropriate */ /************************************************************************/ -void find_file(char *full_path,const char *name, const char *path) +void find_file(char *full_path,const char *name, const char *path, int limit) { const char *p; char *q; - char dir_sep[2]={DIR_SEP,0}; + int l; + + limit--; /* Need one byte for \0 */ for (p=path;p;p=q+1) { q=strchr(p,LIST_SEP); if (q) { - strncpy(full_path,p,q-p); + if (q-p>limit) { + fprintf(stderr, "Avoid buffer overrun path component\n"); + continue; + } + strncpy(full_path,p,q-p); /* Safe because q-p<limit */ full_path[q-p]=0; + l=q-p; /* Length so far is q-p */ } else { q--; - strcpy(full_path,p); + strncpy(full_path,p, limit); + full_path[limit-1]='\0'; /* Ensure null terminated */ + l=strlen(full_path); /* Length so far is length of full_path */ } /* Empty list element means current directory */ if (!*full_path) { full_path[0]='.'; full_path[1]=0; + l=1; /* One character used */ #ifdef __MSDOS__ } else { - strcpy(full_path,add_exe_path(full_path)); + strncpy(full_path,add_exe_path(full_path), limit); + full_path[limit]='\0'; /* Ensure null terminated */ + l=strlen(full_path); /* Length so far is length of full_path */ #endif } - strcat(full_path,dir_sep); - strcat(full_path,name); + if (full_path[l-1]!=DIR_SEP) + { + if (l>limit) + { + fprintf(stderr, "Avoiding buffer overrun path component\n"); + continue; + } + full_path[l]=DIR_SEP; + l++; + } + if (l>limit) { + fprintf(stderr, "Avoiding buffer overrun path component\n"); + continue; + } + strncpy(full_path+l, name, limit-l); if (access(full_path,0)==0) { return; } @@ -58,9 +83,10 @@ void check_charset(char **filename,const char *charset) { char namebuf[128]; char pathbuf[512]; - strcpy(namebuf,charset); - strcat(namebuf,CHARSET_EXT); - find_file(pathbuf,namebuf,charset_path); + strncpy(namebuf,charset, sizeof(namebuf)-4-1); /* Length of CHARSET_EXT is 4 */ + namebuf[sizeof(namebuf)-4]='\0'; /* Ensure null terminated */ + strcat(namebuf,CHARSET_EXT); /* Safe because strncpy leaves enough space */ + find_file(pathbuf,namebuf,charset_path, sizeof(pathbuf)); if (*pathbuf) { *filename=strdup(charset); } @@ -73,7 +99,8 @@ char *exe_dir(void) { static char pathbuf[127]; char *q; - strcpy(pathbuf,_argv[0]); + strncpy(pathbuf,_argv[0], sizeof(pathbuf)-1); + pathbuf[(sizeof(pathbuf)-1]='\0'; /* Ensure null terminated */ q=strrchr(pathbuf,DIR_SEP); if (q) { *q=0; @@ -84,7 +111,7 @@ } char *add_exe_path(const char *name) { static char path[128]; - sprintf(path,name,exe_dir()); + snprintf(path, sizeof(path), name,exe_dir()); return path; } #endif --- src/substmap.c.dist Thu Nov 5 02:46:11 1998 +++ src/substmap.c Thu Nov 5 04:14:36 1998 @@ -23,7 +23,7 @@ char stopchar; int escaped, lineno=0; unsigned int uc; - find_file(fullpath,filename,add_exe_path(map_path)); + find_file(fullpath,filename,add_exe_path(map_path), sizeof(fullpath)); if (!*fullpath) { fprintf(stderr,"Cannot find file %s\n",filename); exit(1); --- src/writer.c.dist Thu Nov 5 02:46:11 1998 +++ src/writer.c Thu Nov 5 04:10:17 1998 @@ -10,33 +10,46 @@ /************************************************************************/ static char buffer[512]=""; void out_char(const char *chunk) { + const char *p; + int l; + if (!wrap_margin) { fputs(chunk,stdout); return; } - strcat(buffer,chunk); - if (strchr(chunk,'\n')) { - /* End of paragraph */ - char *q = map_subst(spec_chars,'\n'); - fputs(buffer,stdout); - *buffer=0; - if (q) fputs(q,stdout); - } else if (strlen(buffer)>wrap_margin) { - char *q=buffer,*p=buffer+wrap_margin; - while (p>buffer&&!isspace(*p)) p--; - if (p==buffer) { - /*worst case - nowhere to wrap. Will use brute force */ - fwrite(buffer,wrap_margin,1,stdout); - fputc('\n',stdout); - p=buffer+wrap_margin; - } else { - *p=0;p++; - fputs(buffer,stdout); - fputc('\n',stdout); - } - for(q=buffer;*p;p++,q++) *q=*p; - *q=0; - } + + p=chunk; + while(*p!='\0') { + l=sizeof(buffer)-strlen(buffer)-1; /* Figure out remaining space */ + if (l>strlen(p)) + l=strlen(p); /* Limit to length of p */ + strncat(buffer, p, l); /* Addend. Max end is sizeof(buffer)-1 */ + buffer[sizeof(buffer)-1]='\0'; /* Ensure terminated */ + p+=l; /* Make p point to remainder of chunk */ + + if (strchr(buffer,'\n')) { + /* End of paragraph */ + char *q = map_subst(spec_chars,'\n'); + fputs(buffer,stdout); + *buffer=0; + if (q) fputs(q,stdout); + } else if (strlen(buffer)>wrap_margin) { + char *q=buffer,*p=buffer+wrap_margin; + while (p>buffer&&!isspace(*p)) p--; + if (p==buffer) { + /*worst case - nowhere to wrap. Will use brute force */ + fwrite(buffer,wrap_margin,1,stdout); + fputc('\n',stdout); + p=buffer+wrap_margin; + } else { + *p=0;p++; + fputs(buffer,stdout); + fputc('\n',stdout); + } + for(q=buffer;*p;p++,q++) *q=*p; + *q=0; + } + } } /**************************************************************************/ /* Converts unicode char to output charset sequence. Coversion have */ @@ -59,7 +72,7 @@ } if ((mapped = map_subst(replacements,uc))) return mapped; if (unknown_as_hex) { - sprintf(hexbuf,"\\x%04X",(unsigned)uc); + snprintf(hexbuf, sizeof(hexbuf), "\\x%04X",(unsigned)uc); /* Be paranoid */ return hexbuf; } return bad_char;