From e6e30e94b1da12d0a3247db3b25311bafb8230e6 Mon Sep 17 00:00:00 2001 From: giraffedata Date: Fri, 9 Aug 2019 03:06:48 +0000 Subject: Remove RLE_CHECK_ALLOC and rle_alloc_error from URT library. This fixes a wild pointer dereference as well as cleans up code git-svn-id: http://svn.code.sf.net/p/netpbm/code/trunk@3656 9d0c8265-081b-0410-96cb-a4ca84ce46f8 --- urt/rle.h | 17 --- urt/rle_error.c | 74 ++++------- urt/rle_getrow.c | 8 +- urt/rle_hdr.c | 394 +++++++++++++++++++++++++++---------------------------- 4 files changed, 219 insertions(+), 274 deletions(-) (limited to 'urt') diff --git a/urt/rle.h b/urt/rle.h index 5d5ff33a..00717748 100644 --- a/urt/rle.h +++ b/urt/rle.h @@ -69,14 +69,6 @@ typedef unsigned short rle_map; */ #define RLE_INIT_MAGIC 0x6487ED51L -/***************************************************************** - * TAG( RLE_CHECK_ALLOC ) - * - * Test for allocation failure, scream and die if so. - */ -#define RLE_CHECK_ALLOC( pgm, ptr, name ) \ - ( !(ptr) ? rle_alloc_error( pgm, name ) : 0 ) - /* * TAG( rle_hdr ) * @@ -163,15 +155,6 @@ extern rle_hdr rle_dflt_hdr; /* Declare RLE library routines. */ -/* From rle_error.c. */ -/***************************************************************** - * TAG( rle_alloc_error ) - * - * Print memory allocation error message and exit. - */ -extern int rle_alloc_error( const char *pgm, - const char *name ); - /***************************************************************** * TAG( rle_get_error ) * diff --git a/urt/rle_error.c b/urt/rle_error.c index 23dd1fd8..801eee17 100644 --- a/urt/rle_error.c +++ b/urt/rle_error.c @@ -30,36 +30,6 @@ #include "rle_config.h" #include "rle.h" -/***************************************************************** - * TAG( rle_alloc_error ) - * - * Print memory allocation error message and exit. - * Inputs: - * pgm: Name of this program. - * name: Name of memory trying to be allocated. - * Outputs: - * Prints message and exits. - * - * Returns int because it's used in a conditional expression. - */ -int -rle_alloc_error( pgm, name ) -CONST_DECL char *pgm, *name; -{ - if ( name ) - fprintf( stderr, "%s: memory allocation failed.\n", pgm ); - else - fprintf( stderr, "%s: memory allocation failed (no space for %s).\n", - pgm, name ); - - exit( RLE_NO_SPACE ); - - /* Will some compilers bitch about this because they know exit - * doesn't return?? - */ - return 0; -} - /***************************************************************** * TAG( rle_get_error ) * @@ -74,44 +44,44 @@ CONST_DECL char *pgm, *name; */ int rle_get_error( code, pgmname, fname ) -int code; -CONST_DECL char *pgmname; -CONST_DECL char *fname; + int code; + CONST_DECL char *pgmname; + CONST_DECL char *fname; { if (! fname || strcmp( fname, "-" ) == 0 ) - fname = "Standard Input"; + fname = "Standard Input"; switch( code ) { case RLE_SUCCESS: /* success */ - break; + break; case RLE_NOT_RLE: /* Not an RLE file */ - fprintf( stderr, "%s: %s is not an RLE file\n", - pgmname, fname ); - break; + fprintf( stderr, "%s: %s is not an RLE file\n", + pgmname, fname ); + break; case RLE_NO_SPACE: /* malloc failed */ - fprintf( stderr, - "%s: Malloc failed reading header of file %s\n", - pgmname, fname ); - break; + fprintf( stderr, + "%s: Malloc failed reading header of file %s\n", + pgmname, fname ); + break; case RLE_EMPTY: - fprintf( stderr, "%s: %s is an empty file\n", - pgmname, fname ); - break; + fprintf( stderr, "%s: %s is an empty file\n", + pgmname, fname ); + break; case RLE_EOF: - fprintf( stderr, - "%s: RLE header of %s is incomplete (premature EOF)\n", - pgmname, fname ); - break; + fprintf( stderr, + "%s: RLE header of %s is incomplete (premature EOF)\n", + pgmname, fname ); + break; default: - fprintf( stderr, "%s: Error encountered reading header of %s\n", - pgmname, fname ); - break; + fprintf( stderr, "%s: Error encountered reading header of %s\n", + pgmname, fname ); + break; } return code; } diff --git a/urt/rle_getrow.c b/urt/rle_getrow.c index 679811cc..ae220f5b 100644 --- a/urt/rle_getrow.c +++ b/urt/rle_getrow.c @@ -102,9 +102,13 @@ rle_get_setup(rle_hdr * const the_hdr) { rle_pixel * bg_color; MALLOCARRAY(the_hdr->bg_color, setup.h_ncolors); + if (!the_hdr->bg_color) + pm_error("Failed to allocation array for %u background colors", + setup.h_ncolors); MALLOCARRAY(bg_color, 1 + (setup.h_ncolors / 2) * 2); - RLE_CHECK_ALLOC(the_hdr->cmd, the_hdr->bg_color && bg_color, - "background color" ); + if (!bg_color) + pm_error("Failed to allocation array for %u background colors", + 1+(setup.h_ncolors / 2) * 2); fread((char *)bg_color, 1, 1 + (setup.h_ncolors / 2) * 2, infile); for (i = 0; i < setup.h_ncolors; ++i) the_hdr->bg_color[i] = bg_color[i]; diff --git a/urt/rle_hdr.c b/urt/rle_hdr.c index 1611324c..1edb7a3f 100644 --- a/urt/rle_hdr.c +++ b/urt/rle_hdr.c @@ -1,298 +1,286 @@ /* * This software is copyrighted as noted below. It may be freely copied, - * modified, and redistributed, provided that the copyright notice is + * modified, and redistributed, provided that the copyright notice is * preserved on all copies. - * + * * There is no warranty or other guarantee of fitness for this software, * it is provided solely "as is". Bug reports or fixes may be sent * to the author, who may or may not act on them as he desires. * * You may not include this software in a program or other software product - * without supplying the source, or without informing the end-user that the + * without supplying the source, or without informing the end-user that the * source is available for no extra charge. * * If you modify this software, you should include a notice giving the * name of the person performing the modification, the date of modification, * and the reason for such modification. */ -/* +/* * rle_hdr.c - Functions to manipulate rle_hdr structures. - * - * Author: Spencer W. Thomas - * EECS Dept. - * University of Michigan - * Date: Mon May 20 1991 + * + * Author: Spencer W. Thomas + * EECS Dept. + * University of Michigan + * Date: Mon May 20 1991 * Copyright (c) 1991, University of Michigan */ #include +#include "nstring.h" +#include "mallocvar.h" + #include "rle_config.h" #include "rle.h" -/***************************************************************** - * TAG( rle_names ) - * + + +void +rle_names(rle_hdr * const hdrP, + const char * const pgmname, + const char * const fname, + int const imgNum) { +/*---------------------------------------------------------------------------- * Load program and file names into header. * Inputs: - * the_hdr: Header to modify. - * pgmname: The program name. - * fname: The file name. - * img_num: Number of the image within the file. + * hdrP: Header to modify. + * pgmname: The program name. + * fname: The file name. + * imgNum: Number of the image within the file. * Outputs: - * the_hdr: Modified header. - * Algorithm: - * If values previously filled in (by testing is_init field), - * free them. Make copies of file name and program name, - * modifying file name for standard i/o. Set is_init field. - */ -void -rle_names( the_hdr, pgmname, fname, img_num ) -rle_hdr *the_hdr; -CONST_DECL char *pgmname; -CONST_DECL char *fname; -int img_num; -{ -#if 0 - /* Can't do this because people do hdr1 = hdr2, which copies - the pointers. */ - - /* If filled in, free previous values. */ - if ( the_hdr->is_init == RLE_INIT_MAGIC && - the_hdr->cmd != NULL && the_hdr->file_name != NULL ) - { - if ( pgmname != the_hdr->cmd ) - free( the_hdr->cmd ); - if ( fname != the_hdr->file_name ) - free( the_hdr->file_name ); - } -#endif + * *hdrP: Modified header. +-----------------------------------------------------------------------------*/ + + /* Algorithm: + If values previously filled in (by testing is_init field), + free them. Make copies of file name and program name, + modifying file name for standard i/o. Set is_init field. + */ + const char * newFname; + const char * newPgmname; /* Mark as filled in. */ - the_hdr->is_init = RLE_INIT_MAGIC; + hdrP->is_init = RLE_INIT_MAGIC; /* Default file name for stdin/stdout. */ - if ( fname == NULL || strcmp( fname, "-" ) == 0 || *fname == '\0' ) - fname = "Standard I/O"; - if ( pgmname == NULL ) - pgmname = rle_dflt_hdr.cmd; + if (!fname || streq(fname, "-") || strlen(fname) == 0) + newFname = "Standard I/O"; + else + newFname = fname; + + if (pgmname) + newPgmname = pgmname; + else + newPgmname = rle_dflt_hdr.cmd; /* Fill in with copies of the strings. */ - if ( the_hdr->cmd != pgmname ) - { - char *tmp = (char *)malloc( strlen( pgmname ) + 1 ); - RLE_CHECK_ALLOC( pgmname, tmp, 0 ); - strcpy( tmp, pgmname ); - the_hdr->cmd = tmp; - } + if (hdrP->cmd != newPgmname) + hdrP->cmd = pm_strdup(newPgmname); - if ( the_hdr->file_name != fname ) - { - char *tmp = (char *)malloc( strlen( fname ) + 1 ); - RLE_CHECK_ALLOC( pgmname, tmp, 0 ); - strcpy( tmp, fname ); - the_hdr->file_name = tmp; - } + if (hdrP->file_name != newFname) + hdrP->cmd = pm_strdup(newFname); - the_hdr->img_num = img_num; + hdrP->img_num = imgNum; } + /* Used by rle_hdr_cp and rle_hdr_init to avoid recursion loops. */ -static int no_recurse = 0; +static int noRecurse = 0; -/***************************************************************** - * TAG( rle_hdr_cp ) - * + + +rle_hdr * +rle_hdr_cp(rle_hdr * const fromHdrP, + rle_hdr * const toHdrArgP) { +/*---------------------------------------------------------------------------- * Make a "safe" copy of a rle_hdr structure. * Inputs: - * from_hdr: Header to be copied. + * *fromHdrP: Header to be copied. * Outputs: - * to_hdr: Copy of from_hdr, with all memory referred to - * by pointers copied. Also returned as function - * value. If NULL, a static header is used. + * *toHdrPd: Copy of from_hdr, with all memory referred to + * by pointers copied. Also returned as function + * value. If NULL, a static header is used. * Assumptions: - * It is safe to call rle_hdr_init on to_hdr. - * Algorithm: - * Initialize to_hdr, copy from_hdr to it, then copy the memory - * referred to by all non-null pointers. - */ -rle_hdr * -rle_hdr_cp( from_hdr, to_hdr ) -rle_hdr *from_hdr, *to_hdr; -{ - static rle_hdr dflt_hdr; - CONST_DECL char *cmd, *file; - int num; + * It is safe to call rle_hdr_init on *toHdrP. +-----------------------------------------------------------------------------*/ + /* Algorithm: + Initialize *toHdrP, copy *fromHdrP to it, then copy the memory + referred to by all non-null pointers. + */ + static rle_hdr dfltHdr; + rle_hdr * toHdrP; + const char * cmd; + const char * file; + unsigned int num; /* Save command, file name, and image number if already initialized. */ - if ( to_hdr && to_hdr->is_init == RLE_INIT_MAGIC ) - { - cmd = to_hdr->cmd; - file = to_hdr->file_name; - num = to_hdr->img_num; - } - else - { - cmd = file = NULL; - num = 0; + if (toHdrArgP && toHdrArgP->is_init == RLE_INIT_MAGIC) { + cmd = toHdrArgP->cmd; + file = toHdrArgP->file_name; + num = toHdrArgP->img_num; + } else { + cmd = file = NULL; + num = 0; } - if ( !no_recurse ) - { - no_recurse++; - rle_hdr_init( to_hdr ); - no_recurse--; + if (!noRecurse) { + ++noRecurse; + rle_hdr_init(toHdrArgP); + --noRecurse; } - if ( to_hdr == NULL ) - to_hdr = &dflt_hdr; + toHdrP = toHdrArgP ? toHdrArgP : &dfltHdr; + + *toHdrP = *fromHdrP; - *to_hdr = *from_hdr; + if (toHdrP->bg_color) { + unsigned int i; - if ( to_hdr->bg_color ) - { - int size = to_hdr->ncolors * sizeof(int); - to_hdr->bg_color = (int *)malloc( size ); - RLE_CHECK_ALLOC( to_hdr->cmd, to_hdr->bg_color, "background color" ); - memcpy( to_hdr->bg_color, from_hdr->bg_color, size ); + MALLOCARRAY(toHdrP->bg_color, toHdrP->ncolors); + if (!toHdrP->bg_color) + pm_error("Failed to allocate array for %u background colors", + toHdrP->ncolors); + for (i = 0; i < toHdrP->ncolors; ++i) + toHdrP->bg_color[i] = fromHdrP->bg_color[i]; } - if ( to_hdr->cmap ) - { - int size = to_hdr->ncmap * (1 << to_hdr->cmaplen) * sizeof(rle_map); - to_hdr->cmap = (rle_map *)malloc( size ); - RLE_CHECK_ALLOC( to_hdr->cmd, to_hdr->cmap, "color map" ); - memcpy( to_hdr->cmap, from_hdr->cmap, size ); + if (toHdrP->cmap) { + size_t const size = + toHdrP->ncmap * (1 << toHdrP->cmaplen) * sizeof(rle_map); + toHdrP->cmap = malloc(size); + if (!toHdrP->cmap) + pm_error("Failed to allocate memory for %u color maps " + "of length %u", toHdrP->ncmap, 1 << toHdrP->cmaplen); + memcpy(toHdrP->cmap, fromHdrP->cmap, size); } /* Only copy array of pointers, as the original comment memory * never gets overwritten. */ - if ( to_hdr->comments ) - { - int size = 0; - CONST_DECL char **cp; - for ( cp=to_hdr->comments; *cp; cp++ ) - size++; /* Count the comments. */ - /* Check if there are really any comments. */ - if ( size ) - { - size++; /* Copy the NULL pointer, too. */ - size *= sizeof(char *); - to_hdr->comments = (CONST_DECL char **)malloc( size ); - RLE_CHECK_ALLOC( to_hdr->cmd, to_hdr->comments, "comments" ); - memcpy( to_hdr->comments, from_hdr->comments, size ); - } - else - to_hdr->comments = NULL; /* Blow off empty comment list. */ + if (toHdrP->comments) { + unsigned int size; + const char ** cp; + + /* Count the comments. */ + for (cp = toHdrP->comments, size = 0; *cp; ++cp) + ++size; + + /* Check if there are really any comments. */ + if (size > 0) { + ++size; /* Copy the NULL pointer, too. */ + size *= sizeof(char *); + toHdrP->comments = malloc(size); + if (!toHdrP->comments) + pm_error("Failed to allocation %u bytes for comments", size); + memcpy(toHdrP->comments, fromHdrP->comments, size); + } else + toHdrP->comments = NULL; /* Blow off empty comment list. */ } /* Restore the names to their original values. */ - to_hdr->cmd = cmd; - to_hdr->file_name = file; + toHdrP->cmd = cmd; + toHdrP->file_name = file; /* Lines above mean nothing much happens if cmd and file are != NULL. */ - rle_names( to_hdr, to_hdr->cmd, to_hdr->file_name, num ); + rle_names(toHdrP, toHdrP->cmd, toHdrP->file_name, num); - return to_hdr; + return toHdrP; } -/***************************************************************** - * TAG( rle_hdr_clear ) - * + + +void +rle_hdr_clear(rle_hdr * const hdrP) { +/*---------------------------------------------------------------------------- * Clear out the allocated memory pieces of a header. * * This routine is intended to be used internally by the library, to * clear a header before putting new data into it. It clears all the * fields that would be set by reading in a new image header. * Therefore, it does not clear the program and file names. - * + * * Inputs: - * the_hdr: To be cleared. + * hdrP: To be cleared. * Outputs: - * the_hdr: After clearing. + * *hdrP: After clearing. * Assumptions: - * If is_init field is RLE_INIT_MAGIC, the header has been - * properly initialized. This will fail every 2^(-32) times, on - * average. - * Algorithm: - * Free memory and set to zero all pointers, except program and - * file name. - */ -void -rle_hdr_clear( the_hdr ) -rle_hdr *the_hdr; -{ + * If is_init field is RLE_INIT_MAGIC, the header has been + * properly initialized. This will fail every 2^(-32) times, on + * average. +-----------------------------------------------------------------------------*/ + /* Algorithm: + Free memory and set to zero all pointers, except program and + file name. + */ + /* Try to free memory. Assume if is_init is properly set that this * header has been previously initialized, therefore it is safe to * free memory. */ - if ( the_hdr && the_hdr->is_init == RLE_INIT_MAGIC ) - { - if ( the_hdr->bg_color ) - free( the_hdr->bg_color ); - the_hdr->bg_color = 0; - if ( the_hdr->cmap ) - free( the_hdr->cmap ); - the_hdr->cmap = 0; - /* Unfortunately, we don't know how to free the comment memory. */ - if ( the_hdr->comments ) - free( the_hdr->comments ); - the_hdr->comments = 0; + if (hdrP && hdrP->is_init == RLE_INIT_MAGIC) { + if (hdrP->bg_color ) + free(hdrP->bg_color); + hdrP->bg_color = NULL; + if (hdrP->cmap ) + free(hdrP->cmap); + hdrP->cmap = NULL; + /* Unfortunately, we don't know how to free the comment memory. */ + if (hdrP->comments) + free(hdrP->comments); + hdrP->comments = NULL; } } -/***************************************************************** - * TAG( rle_hdr_init ) - * +rle_hdr * +rle_hdr_init(rle_hdr * const hdrP) { +/*---------------------------------------------------------------------------- * Initialize a rle_hdr structure. * Inputs: - * the_hdr: Header to be initialized. + * hdrP: Header to be initialized. * Outputs: - * the_hdr: Initialized header. + * *hdrP: Initialized header. * Assumptions: - * If the_hdr->is_init is RLE_INIT_MAGIC, the header has been - * previously initialized. - * If the_hdr is a copy of another rle_hdr structure, the copy - * was made with rle_hdr_cp. - * Algorithm: - * Fill in fields of rle_dflt_hdr that could not be set by the loader - * If the_hdr is rle_dflt_hdr, do nothing else - * Else: - * If the_hdr is NULL, return a copy of rle_dflt_hdr in static storage - * If the_hdr->is_init is RLE_INIT_MAGIC, free all memory - * pointed to by non-null pointers. - * If this is a recursive call to rle_hdr_init, clear *the_hdr and - * return the_hdr. - * Else make a copy of rle_dflt_hdr and return its address. Make the - * copy in static storage if the_hdr is NULL, and in the_hdr otherwise. - */ -rle_hdr * -rle_hdr_init( the_hdr ) -rle_hdr *the_hdr; -{ - rle_hdr *ret_hdr; + * If hdrP->is_init is RLE_INIT_MAGIC, the header has been + * previously initialized. + * If the_hdr is a copy of another rle_hdr structure, the copy + * was made with rle_hdr_cp. +-----------------------------------------------------------------------------*/ + /* Algorithm: + Fill in fields of rle_dflt_hdr that could not be set by the loader + If the_hdr is rle_dflt_hdr, do nothing else + Else: + If hdrP is NULL, return a copy of rle_dflt_hdr in static storage + If hdrP->is_init is RLE_INIT_MAGIC, free all memory + pointed to by non-null pointers. + If this is a recursive call to rle_hdr_init, clear *hdrP and + return hdrP. + Else make a copy of rle_dflt_hdr and return its address. Make the + copy in static storage if hdrP is NULL, and in *hdrP otherwise. + */ + rle_hdr * retval; rle_dflt_hdr.rle_file = stdout; - /* The rest of rle_dflt_hdr is set by the loader's data initialization */ - if ( the_hdr == &rle_dflt_hdr ) - return the_hdr; + /* The rest of rle_dflt_hdr is set by the loader's data initialization */ - rle_hdr_clear( the_hdr ); + if (hdrP == &rle_dflt_hdr) + retval = hdrP; + else { + rle_hdr_clear(hdrP); - /* Only call rle_hdr_cp if not called from there. */ - if ( !no_recurse ) - { - no_recurse++; - ret_hdr = rle_hdr_cp( &rle_dflt_hdr, the_hdr ); - no_recurse--; + /* Call rle_hdr_cp only if not called from there. */ + if (!noRecurse) { + ++noRecurse; + retval = rle_hdr_cp(&rle_dflt_hdr, hdrP); + --noRecurse; + } else + retval = hdrP; } - else - ret_hdr = the_hdr; - - return ret_hdr; + return retval; } + + + -- cgit 1.4.1