From aeefc403ad4b686d3d937865747ed18de040cd4a Mon Sep 17 00:00:00 2001 From: Joachim Schrod Date: Thu, 28 Jun 2007 11:34:05 +0000 Subject: Fix many buffer overflows, caused by unchecked string operations and arbitrary access to arrays. git-svn-id: svn://tug.org/texlive/trunk@4523 c570f23f-e606-0410-a88d-b1316a301751 --- Build/source/texk/dviljk/ChangeLog | 28 ++++- Build/source/texk/dviljk/config.h | 13 +- Build/source/texk/dviljk/dvi2xx.c | 246 ++++++++++++++++++++----------------- Build/source/texk/dviljk/dvi2xx.h | 10 +- 4 files changed, 171 insertions(+), 126 deletions(-) (limited to 'Build/source/texk/dviljk') diff --git a/Build/source/texk/dviljk/ChangeLog b/Build/source/texk/dviljk/ChangeLog index 58a35c725df..08754485510 100644 --- a/Build/source/texk/dviljk/ChangeLog +++ b/Build/source/texk/dviljk/ChangeLog @@ -1,3 +1,23 @@ +2007-06-28 Joachim Schrod + + * dvi2xx.c: Fixed a whole bunch of buffer overflows: The program + did not check memory bounds for any string operation. All places + where strings are copied with strcpy are replaced by dynamically + allocated strings (with xstrdup from kpathsea) or bounded string + operations. Fixed also manual string copy operations on special + strings. Fixed array buffer overflow in defpoint and fill special + operations. + + Bumped version number up to 2.6p3. + + * dvi2xx.h: Some fixed sized string arrays are pointers now, they + get dynamically allocated. + + * config.h: Throw an error if kpathsea is not used. dvi2xx.c + had previously already kpathsea dependencies without protecting + them with #if KPATHSEA. We go that road further since upstream + does not exist any more. + 2006-05-29 Karl Berry * Makefile.in (install-exec): cd $(bindir) in case we are doing @@ -17,7 +37,7 @@ Tue Jan 27 14:32:25 1998 Sebastian Rahtz Olaf Weber Sat Jan 17 15:09:59 1998 Sebastian Rahtz - * assorted patches from Peter Breitenlohner, Olaf Weber, + * assorted patches from Peter Breitenlohner, Olaf Weber, Eli Zaretskii, Fabrice Popineau etc. Hope it all still works. @@ -54,7 +74,7 @@ Sun Jan 5 16:12:06 1997 Karl Berry Mon Dec 9 00:53:30 1996 Ulrik Vieth * dvihp: Use `basename $0` in usage and version messages for - consistency with other programs and scripts. + consistency with other programs and scripts. Also accept -help, -version in addition to --help, --version. Sat Dec 7 17:15:54 1996 Karl Berry @@ -218,7 +238,7 @@ Sat Dec 31 17:30:58 1994 Karl Berry Fri Dec 30 15:25:54 1994 Karl Berry * Makefile.in (install-exec): Don't depend on default, so dvilj4 - doesn't get made if the user didn't want it. + doesn't get made if the user didn't want it. (install-data): mkdirchain on texinputdir and fontdir. From Joachim. @@ -325,7 +345,7 @@ Fri Jul 29 13:26:49 1994 Karl Berry (karl@cs.umb.edu) * config.h (RESOLUTION, MFMODE) [LJ4L]: Set to 300/cx for this new symbol. - * dvi2xx.c (DecodeArgs): Always scale pgsiz_dots. + * dvi2xx.c (DecodeArgs): Always scale pgsiz_dots. (main) [LJ4]: Don't hardwire 600dpi. These fixes from ajp@eng.cam.ac.uk. diff --git a/Build/source/texk/dviljk/config.h b/Build/source/texk/dviljk/config.h index 2c7f7498939..9d9ffb613c6 100644 --- a/Build/source/texk/dviljk/config.h +++ b/Build/source/texk/dviljk/config.h @@ -216,12 +216,7 @@ typedef SCHAR_TYPE signed_char; #endif #ifndef KPATHSEA -extern bool findfile( -#if NeedFunctionPrototypes -char path[], char n[], long4 fontmag, char name[], - bool tfm, int level -#endif - ); +#error "Would need changed findfile, dviljk has changed allocation semantic of name member in tfontptr" #endif @@ -444,3 +439,9 @@ typedef FILE *FILEPTR; /* If we have neither, should fall back to fprintf with fixed args. */ #endif #endif + +#ifndef KPATHSEA +/* FIXME: Should provide a strdup function. But currently this tree is + only used in connection with kpathsea anyhow. */ +#error "Need xstrdup and xmalloc function, e.g. from kpathsea" +#endif diff --git a/Build/source/texk/dviljk/dvi2xx.c b/Build/source/texk/dviljk/dvi2xx.c index 730d616afdb..ac90edd7f5e 100644 --- a/Build/source/texk/dviljk/dvi2xx.c +++ b/Build/source/texk/dviljk/dvi2xx.c @@ -1,5 +1,5 @@ /* $Id: dvi2xx.c,v 2.5 1997/12/08 20:52:20 neumann Exp $ */ -#define VERSION "2.6p2 (dviljk)" +#define VERSION "dviljk (version 2.6p3)" /* #define DEBUGGS 1 */ @@ -173,7 +173,7 @@ char *argv[]; y_origin = YDEFAULTOFF; /* y-origin in dots */ setbuf(ERR_STREAM, NULL); - (void) strcpy(G_progname, argv[0]); + G_progname = argv[0]; #ifdef KPATHSEA kpse_set_progname(argv[0]); kpse_set_program_enabled (kpse_pk_format, MAKE_TEX_PK_BY_DEFAULT, kpse_src_compile); @@ -2968,8 +2968,8 @@ char *argv[]; #endif { int argind; /* argument index for flags */ - char curarea[STRSIZE]; /* current file area */ - char curname[STRSIZE]; /* current file name */ + char *curarea; /* current file area */ + char *curname; /* current file name */ char *tcp, *tcp1; /* temporary character pointers */ char *this_arg; double x_offset = 0.0, y_offset = 0.0; @@ -2988,9 +2988,9 @@ char *argv[]; #endif #endif - if (argc == 2 && (strcmp (argv[1], "--version") == 0)) { + if (argc == 2 && EQ(argv[1], "--version")) { extern KPSEDLL char *kpathsea_version_string; - puts ("dvilj(k) 2.6"); + puts (VERSION); puts (kpathsea_version_string); puts ("Copyright (C) 1997 Gustaf Neumann.\n\ There is NO warranty. You may redistribute this software\n\ @@ -3328,8 +3328,8 @@ Primary author of Dvi2xx: Gustaf Neumann; -k maintainer: K. Berry."); } } else { - (void) strcpy(filename, tcp); - if (!strcmp(filename, "-")) { + filename = tcp; + if (EQ(filename, "-")) { EmitFileName = "-"; #ifdef RISC_USE_OSL dvifp = BINOPEN("Kbd:"); @@ -3339,57 +3339,68 @@ Primary author of Dvi2xx: Gustaf Neumann; -k maintainer: K. Berry."); AssureBinary(fileno(dvifp)); #endif } else { + /* Since this code is used only once during startup, we don't care + about free()ing the allocated strings that represent filenames. + It will be more work to realize proper deallocation handling than + it's worth in terms of saving a few bytes. We consider these + bytes actually static memory where we don't know the size in + advance and don't add them to the allocated_storage count. + [27 Jun 07 -js] */ #ifdef KPATHSEA /* split into directory + file name */ int tcplen, argvlen; tcp = (char *)xbasename(argv[argind]);/* this knows about any kind of slashes */ tcplen = strlen(tcp); + if ( tcplen == 0 ) { + /* This happens when the DVI file name has a trailing slash; this + is not a valid name. Then we terminate the argument parsing + loop, a usage message will be output below. */ + break; + } argvlen = strlen(argv[argind]); if (tcplen == argvlen) - curarea[0] = '\0'; + curarea = ""; else { - (void) strcpy(curarea, argv[argind]); + curarea = xstrdup(argv[argind]); curarea[argvlen-tcplen] = '\0'; } #else tcp = strrchr(argv[argind], '/'); /* split into directory + file name */ if (tcp == NULL) { - curarea[0] = '\0'; + curarea[0] = ""; tcp = argv[argind]; } else { - (void) strcpy(curarea, argv[argind]); + curarea = xstrdup(argv[argind]); curarea[tcp-argv[argind]+1] = '\0'; tcp += 1; } #endif - (void) strcpy(curname, tcp); + curname = (char *) xmalloc(strlen(tcp)+5); /* + space for ".dvi" */ + (void) strcpy(curname, tcp); /* split into file name + extension */ - tcp1 = strrchr(tcp, '.'); + tcp1 = strrchr(curname, '.'); if (tcp1 == NULL) { - (void) strcpy(rootname, curname); + rootname = xstrdup(curname); strcat(curname, ".dvi"); } else { *tcp1 = '\0'; - (void) strcpy(rootname, curname); + rootname = xstrdup(curname); *tcp1 = '.'; } + filename = (char *) xmalloc(strlen(curarea)+strlen(curname)+1); (void) strcpy(filename, curarea); (void) strcat(filename, curname); if ((dvifp = BINOPEN(filename)) == FPNULL) { /* do not insist on .dvi */ if (tcp1 == NULL) { - int l = strlen(curname); - if (l > 4) - curname[l - 4] = '\0'; - l = strlen(filename); - if (l > 4) - filename[l - 4] = '\0'; + filename[strlen(filename) - 4] = '\0'; + dvifp = BINOPEN(filename); } - if (tcp1 != NULL || (dvifp = BINOPEN(filename)) == FPNULL) { + if (dvifp == FPNULL) { #ifdef MSC5 Fatal("%s: can't find DVI file \"%s\"\n\n", G_progname, filename); @@ -3411,7 +3422,7 @@ Primary author of Dvi2xx: Gustaf Neumann; -k maintainer: K. Berry."); y_goffset = (short) MM_TO_PXL(y_offset) + y_origin; if (dvifp == FPNULL) { - fprintf(ERR_STREAM,"\nThis is the DVI to %s converter version %s", + fprintf(ERR_STREAM,"\nThis is the DVI to %s converter %s", PRINTER, VERSION); #ifdef SEVENBIT fprintf(ERR_STREAM,", 7bit"); @@ -3507,13 +3518,8 @@ Primary author of Dvi2xx: Gustaf Neumann; -k maintainer: K. Berry."); exit(1); } if (EQ(EmitFileName, "")) { - if ((EmitFileName = (char *)malloc( STRSIZE )) != NULL) - allocated_storage += STRSIZE; - else - Fatal("Can't allocate storage of %d bytes\n",STRSIZE); - (void) strcpy(EmitFileName, curname); - if ((tcp1 = strrchr(EmitFileName, '.'))) - *tcp1 = '\0'; + EmitFileName = (char *) xmalloc(strlen(rootname)+sizeof(EMITFILE_EXTENSION)); + (void) strcpy(EmitFileName, rootname); strcat(EmitFileName, EMITFILE_EXTENSION); } if (G_quiet) @@ -3895,7 +3901,7 @@ char *str; int n; #endif { - char spbuf[STRSIZE], xs[STRSIZE], ys[STRSIZE]; + char xs[STRSIZE], ys[STRSIZE]; char *sf = NULL, *psfile = NULL; float x,y; long4 x_pos, y_pos; @@ -3903,14 +3909,15 @@ int n; int i, j, j1; static int GrayScale = 10, Pattern = 1; static bool GrayFill = _TRUE; - static long4 p_x[80], p_y[80]; + static long4 p_x[MAX_SPECIAL_DEFPOINTS], p_y[MAX_SPECIAL_DEFPOINTS]; int llx=0, lly=0, urx=0, ury=0, rwi=0, rhi=0; #ifdef WIN32 char *gs_path; #endif str[n] = '\0'; - spbuf[0] = '\0'; + for ( i=0 ; i0) { + if ( j < 0 || j >= MAX_SPECIAL_DEFPOINTS ) { + Warning ("defpoint %d ignored, must be between 0 and %d\n", + j, MAX_SPECIAL_DEFPOINTS); + break; + } x_pos = h; y_pos = v; if (i>1) { @@ -4015,15 +4030,31 @@ int n; if (!kpse_tex_hush ("special")) #endif Warning("invalid point definition\n"); - - sf = NULL; break; case FILL: - (void) strcpy(spbuf, k.Val); - i = sscanf(spbuf,"%d/%d %s",&j,&j1,xs); + /* 254 is STRSIZE-1. cpp should be used to construct that number. */ + i = sscanf(k.Val,"%d/%d %254s",&j,&j1,xs); if (i>1) { #ifdef LJ + if ( j < 0 || j >= MAX_SPECIAL_DEFPOINTS ) { + Warning ("fill ignored, point %d must be between 0 and %d\n", + j, MAX_SPECIAL_DEFPOINTS); + break; + } + if ( p_x[j] == -1 ) { + Warning ("fill ignored, point %d is undefined\n", j); + break; + } + if ( j1 < 0 || j1 >= MAX_SPECIAL_DEFPOINTS ) { + Warning ("fill ignored, point %d must be between 0 and %d\n", + j1, MAX_SPECIAL_DEFPOINTS); + break; + } + if ( p_x[j1] == -1 ) { + Warning ("fill ignored, point %d is undefined\n", j1); + break; + } SetPosn(p_x[j], p_y[j]); x_pos = (long4)PIXROUND(p_x[j1]-p_x[j], hconv); y_pos = (long4)PIXROUND(p_y[j1]-p_y[j], vconv); @@ -4081,6 +4112,9 @@ int n; if (!kpse_tex_hush ("special")) #endif Warning("Invalid keyword or value in \\special - <%s> ignored", k.Key); + + free (k.Key); + if ( k.Val != NULL ) free(k.Val); } if ( sf || psfile ) { @@ -4089,6 +4123,9 @@ int n; PMPflush; #endif if (sf) { + /* -js FIXME: i may not be HPFILE any more if there were any other + keywords in this special! Need to rework the whole file + inclusion code. */ if (i == HPFILE) CopyHPFile( sf ); else @@ -4163,6 +4200,8 @@ int n; } } #endif /* LJ */ + if ( sf != NULL ) free(sf); + if ( psfile != NULL ) free(psfile); } } @@ -4173,12 +4212,11 @@ int n; /**********************************************************************/ /***************************** GetKeyStr ****************************/ /**********************************************************************/ -/* extract first keyword-value pair from string (value part may be null) - * return pointer to remainder of string - * return NULL if none found +/* Extract first keyword-value pair from string (value part may be null), + * keyword and value are allocated and must be free by caller. + * Return pointer to remainder of string, + * return NULL if none found. */ -char KeyStr[STRSIZE]; -char ValStr[STRSIZE]; #if NeedFunctionPrototypes char *GetKeyStr(char *str, KeyWord *kw ) #else @@ -4187,39 +4225,46 @@ char *str; KeyWord *kw; #endif { - char *s, *k, *v, t; + char *s, *start; + char save_char, quote_char; if ( !str ) return( NULL ); for (s = str; *s == ' '; s++) ; /* skip over blanks */ if (*s == '\0') return( NULL ); - for (k = KeyStr; /* extract keyword portion */ - *s != ' ' && *s != '\0' && *s != '='; - *k++ = *s++) - ; - *k = '\0'; - kw->Key = KeyStr; - kw->Val = v = NULL; + start = s++; /* start of keyword */ + while ( *s != ' ' && *s != '\0' && *s != '=' ) /* locate end */ + s++; + save_char = *s; + *s = '\0'; + kw->Key = xstrdup(start); + kw->Val = NULL; kw->vt = None; - for ( ; *s == ' '; s++) - ; /* skip over blanks */ - if ( *s != '=' ) /* look for "=" */ + if ( save_char == '\0' ) /* shortcut when we're at the end */ + return (s); + *s = save_char; /* restore keyword end char */ + while ( *s == ' ' ) s++ ; /* skip over blanks */ + if ( *s != '=' ) /* no "=" means no value */ return( s ); - for (s++; *s == ' '; s++); /* skip over blanks */ - if ( *s == '\'' || *s == '\"' ) /* get string delimiter */ - t = *s++; + for (s++; *s == ' '; s++) + ; /* skip over blanks */ + if ( *s == '\'' || *s == '\"' ) /* get string delimiter */ + quote_char = *s++; else - t = ' '; - for (v = ValStr; /* copy value portion up to delim */ - *s != t && *s != '\0'; - *v++ = *s++) - ; - if ( t != ' ' && *s == t ) - s++; - *v = '\0'; - kw->Val = ValStr; + quote_char = ' '; + start = s; /* no increment, might be "" as value */ + while ( *s != quote_char && *s != '\0' ) + s++; /* locate end of value portion */ + save_char = *s; + *s = '\0'; + kw->Val = xstrdup(start); kw->vt = String; + if ( save_char != '\0' ) { /* save_char is now quote_char */ + *s = save_char; + if ( quote_char != ' ' ) /* we had real quote chars */ + s++; + } return( s ); } @@ -4819,13 +4864,14 @@ struct font_entry *fontptr; the resident fonts. */ if (tfm_read_info(fontptr->n, &tfm_info) && tfm_info.family[0] - && strcmp((char *)tfm_info.family, "HPAUTOTFM") == 0) { + && EQ((char *)tfm_info.family, "HPAUTOTFM")) { unsigned i; double factor = fontptr->s / (double)0x100000; resident_count++; fontptr->resident_p = _TRUE; - strcpy(fontptr->symbol_set, (char *)tfm_info.coding_scheme); + strncpy(fontptr->symbol_set, (char *)tfm_info.coding_scheme, 39); + fontptr->symbol_set[39] = '\0'; fontptr->resid = tfm_info.typeface_id; fontptr->spacing = tfm_info.spacing; fontptr->style = tfm_info.style; @@ -4878,7 +4924,7 @@ struct font_entry *fontptr; fontptr->resident_p = _FALSE; if (tfm_info.family[0] - && strcmp((char *)tfm_info.family, "UNSPECIFIED") == 0) { + && EQ((char *)tfm_info.family, "UNSPECIFIED")) { Warning("font family for %s is UNSPECIFIED; need to run dvicopy?", fontptr->n); fontptr->font_file_id = NO_FILE; @@ -5031,10 +5077,9 @@ printf("[%ld]=%lf * %lf * %lf + 0.5 = %ld\n", if (tfontptr->resident_p) return; - if (!(resident_font_located)) { + if (!(resident_font_located)) #endif -#ifdef KPATHSEA { kpse_glyph_file_type font_ret; char *name; @@ -5047,8 +5092,8 @@ printf("[%ld]=%lf * %lf * %lf + 0.5 = %ld\n", if (name) { font_found = _TRUE; - strcpy (tfontptr->name, name); - free (name); + tfontptr->name = name; + allocated_storage += strlen(name)+1; if (!FILESTRCASEEQ (tfontptr->n, font_ret.name)) { fprintf (stderr, @@ -5071,29 +5116,6 @@ printf("[%ld]=%lf * %lf * %lf + 0.5 = %ld\n", tfontptr->n, dpi); } } -#else /* not KPATHSEA */ - if (!(findfile(PXLpath, - tfontptr->n, - tfontptr->font_mag, - tfontptr->name, - _FALSE, - 0))) { - Warning(tfontptr->name); /* contains error messsage */ - tfontptr->font_file_id = NO_FILE; -#ifdef __riscos - MakeMetafontFile(PXLpath, tfontptr->n, tfontptr->font_mag); -#endif - } - else { - font_found = _TRUE; - if (G_verbose) - fprintf(ERR_STREAM,"%d: using font <%s>\n", plusid, tfontptr->name); - } -#endif /* not KPATHSEA */ - -#ifdef LJ_RESIDENT_FONTS - } -#endif tfontptr->plusid = plusid; plusid++; diff --git a/Build/source/texk/dviljk/dvi2xx.h b/Build/source/texk/dviljk/dvi2xx.h index 0f052495104..47fc31b36f4 100644 --- a/Build/source/texk/dviljk/dvi2xx.h +++ b/Build/source/texk/dviljk/dvi2xx.h @@ -116,6 +116,7 @@ #define HUGE_SIZE (unsigned char) 2 #define HUGE_CHAR_PATTERN 32767l #define BYTES_PER_PIXEL_LINE 500 /* max number of bytes per pixel line */ +#define MAX_SPECIAL_DEFPOINTS 80 /* max number of defpoint specials */ #define PK_POST 245 @@ -307,6 +308,7 @@ int printf(); int sscanf(); int strcmp(); char *strcpy(); +char *strncpy(); # ifdef MSC5 unsigned int strlen(); # endif @@ -393,7 +395,7 @@ struct font_entry { /* font entry */ char n[STRSIZE]; /* FNT_DEF command parameters */ long4 font_mag; /* computed from FNT_DEF s and d parameters */ /*char psname[STRSIZE];*/ /* PostScript name of the font */ - char name[STRSIZE]; /* full name of PXL file */ + char *name; /* full name of PXL file */ FILEPTR font_file_id; /* file identifier (NO_FILE if none) */ #ifdef USEPXL long4 magnification; /* magnification read from PXL file */ @@ -534,9 +536,9 @@ bool LastPageSpecified = _FALSE; #ifndef KPATHSEA char *PXLpath = FONTAREA; #endif -char G_progname[STRSIZE]; /* program name */ -char filename[STRSIZE]; /* DVI file name */ -char rootname[STRSIZE]; /* DVI filename without extension */ +char *G_progname; /* program name */ +char *filename; /* DVI file name */ +char *rootname; /* DVI filename without extension */ char *HeaderFileName = ""; /* file name & path of Headerfile */ char *EmitFileName = ""; /* file name & path for output */ #ifdef IBM3812 -- cgit v1.2.3