[GRASS-dev] Re: [bug #1140] (grass) Non-portable snprintf() used in
several programs
Paul Kelly
paul-grass at stjohnspoint.co.uk
Wed Jul 5 07:11:59 EDT 2006
On Tue, 4 Jul 2006, Markus Neteler via RT wrote:
> the snprintf() is used as of today in:
>
> ./raster3d/r3.in.ascii/main.c
> ./db/drivers/dbf/dbfexe.c
> ./raster/r.support/front/front.c
> ./raster/r.support/front/check.c
> ./raster/r.support/front/run.c
> ./raster/r.support/modhead/check_un.c
> ./raster/r.support/modhead/modhead.c
> ./raster/r.support/modhead/ask_format.c
> ./lib/init/clean_temp.c
> ./lib/db/dbmi_client/select.c
> ./lib/gis/user_config.c
> ./lib/vector/dglib/examples/components.c
Please see attached patch to replace use of snprintf in all those
modules. I am posting it to the list first, hopefully for some comments,
before committing. Have to admit what I was doing felt a bit pedantic
towards the end. But some comments:
Mostly what I have done is replaced snprintf writing into fixed size
buffers by dynamically allocating space for the buffers based on the
length of the strings that were being copied in. Where a formatted number
was going in I allowed 32 characters to be on the safe side.
In all but one example (./lib/init/clean_temp.c) snprintf() was being used
without a check on the return value. So while a buffer overflow may have
been avoided, if the string was truncated it would lead to unpredictable
results later in the program. This won't happen now.
Also in another case (./lib/gis/user_config.c) snprintf() was used with a
buffer that had already been correctly dynamically sized, so it wasn't
even needed there.
Paul
-------------- next part --------------
Index: db/drivers/dbf/dbfexe.c
===================================================================
RCS file: /grassrepository/grass6/db/drivers/dbf/dbfexe.c,v
retrieving revision 1.36
diff -u -r1.36 dbfexe.c
--- db/drivers/dbf/dbfexe.c 9 Feb 2006 03:08:48 -0000 1.36
+++ db/drivers/dbf/dbfexe.c 5 Jul 2006 11:02:49 -0000
@@ -353,11 +353,11 @@
if( val->type == SQLP_I ){
val->d = (double)val->i;
val->s = (char*)G_malloc (32*sizeof(char));
- snprintf( val->s, 32*sizeof(char), "%d", val->i );
+ sprintf( val->s, "%d", val->i );
}else if( val->type == SQLP_D ){
val->i = (int)val->d;
val->s = (char*)G_malloc (32*sizeof(char));
- snprintf( val->s, 32*sizeof(char), "%g", val->d );
+ sprintf( val->s, "%g", val->d );
}else if( val->type == SQLP_S ){
val->i = atoi( val->s );
val->d = atof( val->s );
Index: lib/db/dbmi_client/select.c
===================================================================
RCS file: /grassrepository/grass6/lib/db/dbmi_client/select.c,v
retrieving revision 1.7
diff -u -r1.7 select.c
--- lib/db/dbmi_client/select.c 30 May 2006 04:06:59 -0000 1.7
+++ lib/db/dbmi_client/select.c 5 Jul 2006 11:02:49 -0000
@@ -49,7 +49,7 @@
{
int type, more, alloc, count;
int *val;
- char buf[1024], *sval;
+ char *buf, *sval;
dbString stmt;
dbCursor cursor;
dbColumn *column;
@@ -62,15 +62,20 @@
alloc = 1000;
val = (int *) G_malloc ( alloc * sizeof(int));
- if ( where == NULL || strlen(where) == 0 )
- snprintf(buf,1023, "SELECT %s FROM %s", col, tab);
- else
- snprintf(buf,1023, "SELECT %s FROM %s WHERE %s", col, tab, where);
+ if ( where == NULL || strlen(where) == 0 ) {
+ buf = G_malloc(strlen(col) + strlen(tab) + 14);
+ sprintf(buf, "SELECT %s FROM %s", col, tab);
+ }
+ else {
+ buf = G_malloc(strlen(col) + strlen(tab) + strlen(where) + 21);
+ sprintf(buf, "SELECT %s FROM %s WHERE %s", col, tab, where);
+ }
G_debug (3, " SQL: %s", buf );
db_init_string ( &stmt);
db_append_string ( &stmt, buf);
+ G_free(buf);
if (db_open_select_cursor(driver, &stmt, &cursor, DB_SEQUENTIAL) != DB_OK)
return (-1);
Index: lib/gis/user_config.c
===================================================================
RCS file: /grassrepository/grass6/lib/gis/user_config.c,v
retrieving revision 2.4
diff -u -r2.4 user_config.c
--- lib/gis/user_config.c 16 Feb 2006 06:08:49 -0000 2.4
+++ lib/gis/user_config.c 5 Jul 2006 11:02:49 -0000
@@ -76,7 +76,7 @@
if ( NULL == ( path = G_calloc ( 1, len ) ) ) {
return NULL;
}
- snprintf ( path, len, "%s%s", homedir, "/.grass" );
+ sprintf ( path, "%s%s", homedir, "/.grass" );
#else
me = getuid();
my_passwd = getpwuid (me);
@@ -87,7 +87,7 @@
if (NULL == (path = G_calloc (1, len)))
return NULL;
- snprintf (path, len, "%s%s", my_passwd->pw_dir, "/.grass");
+ sprintf (path, "%s%s", my_passwd->pw_dir, "/.grass");
#endif
status = lstat (path, &buf);
Index: lib/init/clean_temp.c
===================================================================
RCS file: /grassrepository/grass6/lib/init/clean_temp.c,v
retrieving revision 2.5
diff -u -r2.5 clean_temp.c
--- lib/init/clean_temp.c 24 Jun 2006 19:33:03 -0000 2.5
+++ lib/init/clean_temp.c 5 Jul 2006 11:02:49 -0000
@@ -19,7 +19,6 @@
*
* 2006: Rewritten for GRASS 6 by roberto Flor, ITC-irst
*
- * TODO (?): Implement snprintf() as G_snprintf() for portability
**************************************************************/
#include <limits.h>
@@ -39,7 +38,6 @@
void clean_dir(const char *pathname,uid_t uid,pid_t pid,time_t now,int max_age)
{
- char buf[BUF_MAX];
DIR *curdir;
struct dirent *cur_entry;
struct stat info;
@@ -53,11 +51,16 @@
/* loop over current dir */
while ((cur_entry = readdir(curdir)))
{
- if ((G_strcasecmp(cur_entry->d_name,".") == 0 )|| (G_strcasecmp(cur_entry->d_name,"..")==0))
+ static char *buf = NULL;
+
+ if ((G_strcasecmp(cur_entry->d_name,".") == 0 )|| (G_strcasecmp(cur_entry->d_name,"..")==0))
continue; /* Skip dir and parent dir entries */
-
- if ( (pathlen=snprintf(buf,BUF_MAX,"%s/%s",pathname,cur_entry->d_name)) >= BUF_MAX)
- G_fatal_error("clean_temp: exceeded maximum pathname length %d, got %d, should'nt happen",BUF_MAX,pathlen);
+
+ if(buf)
+ G_free(buf);
+
+ buf = G_malloc(strlen(pathname) + strlen(cur_entry->d_name) + 2);
+ sprintf(buf, "%s/%s", pathname, cur_entry->d_name);
if (stat(buf, &info) != 0) {
G_warning("Can't stat file %s: %s,skipping\n",buf,strerror(errno));
Index: lib/vector/dglib/examples/components.c
===================================================================
RCS file: /grassrepository/grass6/lib/vector/dglib/examples/components.c,v
retrieving revision 1.4
diff -u -r1.4 components.c
--- lib/vector/dglib/examples/components.c 3 Jan 2006 08:50:48 -0000 1.4
+++ lib/vector/dglib/examples/components.c 5 Jul 2006 11:02:49 -0000
@@ -50,7 +50,7 @@
#define MY_MAX_COMPONENTS 1024
dglGraph_s agraphComponents[MY_MAX_COMPONENTS];
int nret , fd , i , cComponents;
- char szGraphOutFilename[1024];
+ char *pszGraphOutFilename;
/* program options
*/
@@ -109,11 +109,14 @@
if ( dglGet_EdgeCount( & agraphComponents[i] ) > 0 ) {
if ( pszGraphOut ) {
- snprintf( szGraphOutFilename, sizeof(szGraphOutFilename), "%s-component-%d", pszGraphOut, i );
- printf( "[write <%s>...", szGraphOutFilename ); fflush(stdout);
- if ( (fd = open( szGraphOutFilename , O_WRONLY | O_CREAT | O_TRUNC, 0666 )) < 0 ) {
+ pszGraphOutFilename = G_malloc(strlen(pszGraphOut) + 12 + 32);
+ sprintf( pszGraphOutFilename, "%s-component-%d", pszGraphOut, i );
+ printf( "[write <%s>...", pszGraphOutFilename ); fflush(stdout);
+ if ( (fd = open( pszGraphOutFilename , O_WRONLY | O_CREAT | O_TRUNC, 0666 )) < 0 ) {
perror( "open" ); return 1;
}
+ else
+ G_free(pszGraphOutFilename);
dglWrite( & agraphComponents[i], fd );
if ( nret < 0 ) {
fprintf( stderr , "dglWrite error: %s\n" , dglStrerror( & graph ) );
Index: raster/r.support/front/check.c
===================================================================
RCS file: /grassrepository/grass6/raster/r.support/front/check.c,v
retrieving revision 1.3
diff -u -r1.3 check.c
--- raster/r.support/front/check.c 9 Feb 2006 03:09:03 -0000 1.3
+++ raster/r.support/front/check.c 5 Jul 2006 11:02:49 -0000
@@ -1,4 +1,5 @@
#include <stdlib.h>
+#include <string.h>
#include <grass/gis.h>
#include <grass/glocale.h>
#include "local_proto.h"
@@ -19,17 +20,25 @@
int i;
int cats_ok;
int max;
- char question[100];
-
+ char *buff;
+
/* NOTE: G_yes() return value 1 = hitreturn(), 0 otherwise */
data_type = G_raster_map_type(name, mapset);
/* Exit if not updating statistics */
- snprintf(question, sizeof(question), _("Update the statistics "
- "(histogram, range) for [%s]? "), name);
- if (!G_yes(question, 0))
+ {
+ const char *question = _("Update the statistics "
+ "(histogram, range) for [%s]? ");
+
+ buff = G_malloc(strlen(question) + strlen(name) - 1);
+ sprintf(buff, question, name);
+ }
+
+ if (!G_yes(buff, 0))
return EXIT_FAILURE;
+ else
+ G_free(buff);
G_message(_("\n Updating statistics for [%s]"), name);
if (!do_histogram(name, mapset))
Index: raster/r.support/front/front.c
===================================================================
RCS file: /grassrepository/grass6/raster/r.support/front/front.c,v
retrieving revision 1.6
diff -u -r1.6 front.c
--- raster/r.support/front/front.c 27 Mar 2006 08:13:58 -0000 1.6
+++ raster/r.support/front/front.c 5 Jul 2006 11:02:49 -0000
@@ -39,7 +39,6 @@
struct Cell_head cellhd;
struct GModule *module;
struct Option *raster, *title_opt, *history_opt;
- char element[255];
char buf[512];
int cellhd_ok; /* Is cell header OK? */
int is_reclass; /* Is raster reclass? */
@@ -178,6 +177,7 @@
unsigned char *null_bits;
int row, col;
int null_fd;
+ char *element;
if (is_reclass)
G_fatal_error(_("[%s] is a reclass of another map. Exiting."), raster->answer);
@@ -189,8 +189,10 @@
null_bits[col] = 0;
/* Open null file for writing */
+ element = G_malloc(strlen(raster->answer) + 11);
sprintf(element, "cell_misc/%s", raster->answer);
null_fd = G_open_new(element, "null");
+ G_free(element);
G_message(_("Writing new null file for [%s]... "), raster->answer);
for (row = 0; row < cellhd.rows; row++) {
@@ -212,7 +214,8 @@
"(all zero cells will then be considered no data)? "), raster->answer);
if (G_yes(buf, 0)) {
int null_fd;
- char path[400];
+ char path[1024];
+ char *element;
if (is_reclass)
G_fatal_error(_("[%s] is a reclass of another map. Exiting."), raster->answer);
@@ -222,10 +225,12 @@
/* Write a file of no-nulls */
G_message(_("Removing null file for [%s]...\n"), raster->answer);
- snprintf(element, sizeof(element), "cell_misc/%s", raster->answer);
+ element = G_malloc(strlen(raster->answer) + 11);
+ sprintf(element, "cell_misc/%s", raster->answer);
null_fd = G_open_new(element, "null");
G__file_name(path, element, "null", mapset);
unlink(path);
+ G_free(element);
close(null_fd);
G_done_msg(_("Done."));
Index: raster/r.support/front/run.c
===================================================================
RCS file: /grassrepository/grass6/raster/r.support/front/run.c,v
retrieving revision 1.4
diff -u -r1.4 run.c
--- raster/r.support/front/run.c 9 Feb 2006 03:09:03 -0000 1.4
+++ raster/r.support/front/run.c 5 Jul 2006 11:02:49 -0000
@@ -1,5 +1,6 @@
#include <unistd.h>
#include <stdlib.h>
+#include <string.h>
#include <grass/gis.h>
@@ -10,14 +11,17 @@
*/
int run_etc_support(char *pgm, char *rast)
{
- char buf[1024];
+ const char *gisbase = G_gisbase();
+ char *buf;
int stat;
- snprintf(buf, sizeof(buf), "%s/etc/support/%s '%s'",
- G_gisbase(), pgm, rast);
+ buf = G_malloc(strlen(gisbase) + strlen(pgm) + strlen(rast) + 17);
+ sprintf(buf, "%s/etc/support/%s '%s'", gisbase, pgm, rast);
if ((stat = G_system(buf)))
G_sleep(3);
+
+ G_free(buf);
return stat;
}
@@ -30,11 +34,9 @@
*/
int run_system(char *pgm)
{
- char buf[1024];
int stat;
- snprintf(buf, sizeof(buf), "%s", pgm);
- if ((stat = G_system(buf)))
+ if ((stat = G_system(pgm)))
G_sleep(3);
return stat;
Index: raster/r.support/modhead/ask_format.c
===================================================================
RCS file: /grassrepository/grass6/raster/r.support/modhead/ask_format.c,v
retrieving revision 1.2
diff -u -r1.2 ask_format.c
--- raster/r.support/modhead/ask_format.c 9 Feb 2006 03:09:03 -0000 1.2
+++ raster/r.support/modhead/ask_format.c 5 Jul 2006 11:02:49 -0000
@@ -15,17 +15,23 @@
{
RASTER_MAP_TYPE maptype;
char title[80];
- char buf[80];
+ char *buf;
char no_zeros[80];
G_zero(no_zeros, (int)sizeof(no_zeros));
maptype = G_raster_map_type(name, G_mapset());
- snprintf(title, sizeof(title), _("Please enter the following "
- "information for [%s]:"), name);
+ {
+ const char *question = _("Please enter the following "
+ "information for [%s]:");
+
+ buf = G_malloc(strlen(question) + strlen(name) - 1);
+ sprintf(buf, question, name);
+ }
V_clear();
- V_line(0, title);
+ V_line(0, buf);
+ G_free(buf);
V_line(2, _(" Number of rows"));
V_line(3, _(" Number of cols"));
V_line(4, (maptype == CELL_TYPE ?
@@ -46,9 +52,16 @@
if (maptype == CELL_TYPE && cellhd->compressed == 0 &&
cellhd->rows * cellhd->cols * cellhd->format != filesize) {
- snprintf(buf, sizeof(buf), _("rows * cols * bytes per cell "
- "must be same as file size (%ld)"), filesize);
+ {
+ const char *question = _("rows * cols * bytes per cell "
+ "must be same as file size (%ld)");
+
+ buf = G_malloc(strlen(question) + 32);
+ sprintf(buf, question, filesize);
+ }
+
V_line(6, buf);
+ G_free(buf);
V_line(7, _("If you need help figuring them out, just hit ESC"));
}
V_line(10, no_zeros);
@@ -72,7 +85,7 @@
break;
strcpy(no_zeros, _("** Negative values not allowed!"));
- } else
+ } else
strcpy(no_zeros, _("** Positive values only please!"));
}
Index: raster/r.support/modhead/check_un.c
===================================================================
RCS file: /grassrepository/grass6/raster/r.support/modhead/check_un.c,v
retrieving revision 1.3
diff -u -r1.3 check_un.c
--- raster/r.support/modhead/check_un.c 9 Feb 2006 03:09:03 -0000 1.3
+++ raster/r.support/modhead/check_un.c 5 Jul 2006 11:02:49 -0000
@@ -19,7 +19,7 @@
long filesize_calc;
FILE *fd;
static char *tempfile = NULL;
- char command[256];
+ char *command;
/* Check for bad file size */
filesize_calc = cellhd->rows * cellhd->cols * cellhd->format;
@@ -59,8 +59,10 @@
fclose(fd);
/* display valid combinations */
- snprintf(command, sizeof(command), "$GRASS_PAGER %s", tempfile);
+ command = G_malloc(strlen(tempfile) + 14);
+ sprintf(command, "$GRASS_PAGER %s", tempfile);
G_system(command);
+ G_free(command);
/* remove temp file */
unlink(tempfile);
Index: raster/r.support/modhead/modhead.c
===================================================================
RCS file: /grassrepository/grass6/raster/r.support/modhead/modhead.c,v
retrieving revision 1.4
diff -u -r1.4 modhead.c
--- raster/r.support/modhead/modhead.c 9 Feb 2006 03:09:03 -0000 1.4
+++ raster/r.support/modhead/modhead.c 5 Jul 2006 11:02:49 -0000
@@ -197,7 +197,15 @@
/* If we create a new cell header, find out if file is compressed */
if (!cellhd_ok) {
- snprintf(buffer, sizeof(buffer), _("[%s] appears to be compressed. Is it? "), name);
+ char *buff;
+
+ {
+ const char *query = _("[%s] appears to be compressed. Is it? ");
+
+ buff = G_malloc(strlen(query) + strlen(name) - 1);
+ sprintf(buff, query, name);
+ }
+
cellhd.compressed = 0;
if ((compressed_new || compressed_old) && G_yes(buffer, -1)) {
@@ -236,6 +244,7 @@
cellhd.rows = rows_old;
}
}
+ G_free(buff);
} else {
if ((cellhd.compressed < 0) && !compressed_old)
cellhd.compressed = 0;
Index: raster3d/r3.in.ascii/main.c
===================================================================
RCS file: /grassrepository/grass6/raster3d/r3.in.ascii/main.c,v
retrieving revision 2.3
diff -u -r2.3 main.c
--- raster3d/r3.in.ascii/main.c 9 Feb 2006 03:09:04 -0000 2.3
+++ raster3d/r3.in.ascii/main.c 5 Jul 2006 11:02:49 -0000
@@ -120,11 +120,14 @@
readHeaderString (FILE *fp, char *valueString, double *value)
{
- static char format[100];
+ char *format;
- snprintf (format, 100, "%s %%lf", valueString); /*to avoid bufferoverflows we use snprintf*/
+ format = G_malloc(strlen(valueString) + 5);
+ sprintf(format, "%s %%lf", valueString);
if (fscanf (fp, format, value) != 1)
fatalError ("readHeaderString: header value invalid");
+
+ G_free(format);
while (fgetc (fp) != '\n');
}
More information about the grass-dev
mailing list