TUCoPS :: Unix :: General
:: catdoc-1.htm
Catdoc-0.90 many buffer overruns & simple patch
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;