[PATCH:libX11 02/12] xlibi18n: convert sprintf calls to snprintf

Alan Coopersmith alan.coopersmith at oracle.com
Sat Aug 10 13:54:59 PDT 2013


Previous code seemed to assume that printf("%s", NULL) would result
in a 0-length string, not "(null)" or similar, but since there's no
point looking for files in "(null)/filepath...", instead we just
skip over NULL entries in search paths when generating file names.

Signed-off-by: Alan Coopersmith <alan.coopersmith at oracle.com>
---
 src/xlibi18n/XlcDL.c     |    7 +-----
 src/xlibi18n/lcDynamic.c |    4 ++--
 src/xlibi18n/lcFile.c    |   45 +++++++++++++++++++----------------
 src/xlibi18n/lcGeneric.c |   58 ++++++++++++++++++++++++----------------------
 src/xlibi18n/lcUTF8.c    |    8 +++----
 5 files changed, 62 insertions(+), 60 deletions(-)

diff --git a/src/xlibi18n/XlcDL.c b/src/xlibi18n/XlcDL.c
index 79e8a2f..02860a0 100644
--- a/src/xlibi18n/XlcDL.c
+++ b/src/xlibi18n/XlcDL.c
@@ -189,12 +189,7 @@ resolve_object(char *path, const char *lc_name)
 	  Xmalloc(sizeof(XI18NObjectsListRec) * lc_len);
       if (!xi18n_objects_list) return;
     }
-/*
-1266793
-Limit the length of path to prevent stack buffer corruption.
-    sprintf(filename, "%s/%s", path, "XI18N_OBJS");
-*/
-    sprintf(filename, "%.*s/%s", BUFSIZ - 12, path, "XI18N_OBJS");
+    snprintf(filename, sizeof(filename), "%s/%s", path, "XI18N_OBJS");
     fp = fopen(filename, "r");
     if (fp == (FILE *)NULL){
 	return;
diff --git a/src/xlibi18n/lcDynamic.c b/src/xlibi18n/lcDynamic.c
index f6df94c..3821bff 100644
--- a/src/xlibi18n/lcDynamic.c
+++ b/src/xlibi18n/lcDynamic.c
@@ -65,8 +65,8 @@ _XlcDynamicLoader(
     XLCd lcd;
     void *nlshandler;
 
-    sprintf(libpath,"%s/%s/%s",
-		XLOCALEDIR,name,LCLIBNAME);
+    snprintf(libpath, sizeof(libpath), "%s/%s/%s",
+	     XLOCALEDIR, name, LCLIBNAME);
     nlshandler = dlopen(libpath,LAZY);
     _XlcGenericMethods = (XLCdMethods)dlsym(nlshandler,"genericMethods");
     lcd = _XlcCreateLC(name,_XlcGenericMethods);
diff --git a/src/xlibi18n/lcFile.c b/src/xlibi18n/lcFile.c
index 61a14e7..bd2972a 100644
--- a/src/xlibi18n/lcFile.c
+++ b/src/xlibi18n/lcFile.c
@@ -486,9 +486,11 @@ _XlcFileName(
     for (i = 0; i < n; ++i) {
 	char buf[PATH_MAX], *name;
 
+	if (args[i] == NULL)
+	    continue;
+
 	name = NULL;
-	if ((5 + (args[i] ? strlen (args[i]) : 0) + strlen(cat)) < PATH_MAX) {
-	    sprintf(buf, "%s/%s.dir", args[i], cat);
+	if (snprintf(buf, PATH_MAX, "%s/%s.dir", args[i], cat) < PATH_MAX) {
 	    name = resolve_name(siname, buf, RtoL);
 	}
 	if (name == NULL) {
@@ -498,13 +500,13 @@ _XlcFileName(
 	    /* supposed to be absolute path name */
 	    file_name = name;
 	} else {
-	    file_name = Xmalloc(2 + (args[i] ? strlen (args[i]) : 0) +
-				(name ? strlen (name) : 0));
-	    if (file_name != NULL)
-		sprintf(file_name, "%s/%s", args[i], name);
+	    if (snprintf(buf, PATH_MAX, "%s/%s", args[i], name) < PATH_MAX)
+		file_name = strdup(buf);
+	    else
+		file_name = NULL;
 	    Xfree(name);
 	}
-	if (isreadable(file_name)) {
+	if (file_name && isreadable(file_name)) {
 	    break;
 	}
 	Xfree(file_name);
@@ -535,9 +537,11 @@ _XlcResolveLocaleName(
     xlocaledir (dir, PATH_MAX);
     n = _XlcParsePath(dir, args, NUM_LOCALEDIR);
     for (i = 0; i < n; ++i) {
-	if ((2 + (args[i] ? strlen (args[i]) : 0) +
-	    strlen (locale_alias)) < PATH_MAX) {
-	    sprintf (buf, "%s/%s", args[i], locale_alias);
+	if (args[i] == NULL)
+	    continue;
+
+	if (snprintf (buf, PATH_MAX, "%s/%s", args[i], locale_alias)
+	    < PATH_MAX) {
 	    name = resolve_name (lc_name, buf, LtoR);
 	    if (!name) {
 		if (!nlc_name)
@@ -634,9 +638,11 @@ _XlcLocaleDirName(char *dir_name, size_t dir_len, char *lc_name)
     n = _XlcParsePath(dir, args, 256);
     for (i = 0; i < n; ++i) {
 
-	if ((2 + (args[i] ? strlen(args[i]) : 0) +
- 	     strlen(locale_alias)) < PATH_MAX) {
- 	    sprintf (buf, "%s/%s", args[i], locale_alias);
+	if (args[i] == NULL)
+	    continue;
+
+	if (snprintf (buf, PATH_MAX, "%s/%s", args[i], locale_alias)
+	    < PATH_MAX) {
  	    name = resolve_name(lc_name, buf, LtoR);
 	    if (!name) {
 		if (!nlc_name)
@@ -659,8 +665,7 @@ _XlcLocaleDirName(char *dir_name, size_t dir_len, char *lc_name)
  		Xfree(name);
  	    continue;
  	}
- 	if ((1 + strlen (target_dir) + strlen("locale.dir")) < PATH_MAX) {
- 	    sprintf(buf, "%s/locale.dir", target_dir);
+	if (snprintf(buf, PATH_MAX, "%s/locale.dir", target_dir) < PATH_MAX) {
  	    target_name = resolve_name(name, buf, RtoL);
  	}
  	if (name != lc_name)
@@ -731,10 +736,11 @@ _XlcLocaleLibDirName(char *dir_name, size_t dir_len, char *lc_name)
     xlocalelibdir (dir, PATH_MAX);
     n = _XlcParsePath(dir, args, 256);
     for (i = 0; i < n; ++i) {
+	if (args[i] == NULL)
+	    continue;
 
-	if ((2 + (args[i] ? strlen(args[i]) : 0) +
- 	     strlen(locale_alias)) < PATH_MAX) {
- 	    sprintf (buf, "%s/%s", args[i], locale_alias);
+	if (snprintf (buf, PATH_MAX, "%s/%s", args[i], locale_alias)
+	    < PATH_MAX) {
  	    name = resolve_name(lc_name, buf, LtoR);
 	    if (!name) {
 		if (!nlc_name)
@@ -757,8 +763,7 @@ _XlcLocaleLibDirName(char *dir_name, size_t dir_len, char *lc_name)
  		Xfree(name);
  	    continue;
  	}
- 	if ((1 + strlen (target_dir) + strlen("locale.dir")) < PATH_MAX) {
- 	    sprintf(buf, "%s/locale.dir", target_dir);
+	if (snprintf(buf, PATH_MAX, "%s/locale.dir", target_dir) < PATH_MAX) {
  	    target_name = resolve_name(name, buf, RtoL);
  	}
  	if (name != lc_name)
diff --git a/src/xlibi18n/lcGeneric.c b/src/xlibi18n/lcGeneric.c
index 619cb47..f604fe2 100644
--- a/src/xlibi18n/lcGeneric.c
+++ b/src/xlibi18n/lcGeneric.c
@@ -428,17 +428,17 @@ read_charset_define(
 
     for (i=0; ; i++) { /* loop start */
         charsetd = 0;
-        sprintf(csd, "csd%d", i);
+        snprintf(csd, sizeof(csd), "csd%d", i);
 
         /* charset_name  */
-        sprintf(name, "%s.%s", csd, "charset_name");
+        snprintf(name, sizeof(name), "%s.%s", csd, "charset_name");
         _XlcGetResource(lcd, "XLC_CHARSET_DEFINE", name, &value, &num);
         _XlcDbg_printValue(name,value,num);
         if (num > 0) {
 	    /* hackers will get truncated -- C'est la vie */
             strncpy(cset_name,value[0], sizeof cset_name - 1);
 	    cset_name[(sizeof cset_name) - 1] = '\0';
-            sprintf(name, "%s.%s", csd , "side");
+            snprintf(name, sizeof(name), "%s.%s", csd , "side");
             _XlcGetResource(lcd, "XLC_CHARSET_DEFINE", name, &value, &num);
             if (num > 0) {
                 _XlcDbg_printValue(name,value,num);
@@ -470,21 +470,21 @@ read_charset_define(
         /* side   */
         charsetd->side = side ;
         /* length */
-        sprintf(name, "%s.%s", csd, "length");
+        snprintf(name, sizeof(name), "%s.%s", csd, "length");
         _XlcGetResource(lcd, "XLC_CHARSET_DEFINE", name, &value, &num);
         if (num > 0) {
             _XlcDbg_printValue(name,value,num);
             charsetd->char_size = atoi(value[0]);
         }
         /* gc_number */
-        sprintf(name, "%s.%s", csd, "gc_number");
+        snprintf(name, sizeof(name), "%s.%s", csd, "gc_number");
         _XlcGetResource(lcd, "XLC_CHARSET_DEFINE", name, &value, &num);
         if (num > 0) {
             _XlcDbg_printValue(name,value,num);
             charsetd->set_size = atoi(value[0]);
         }
         /* string_encoding */
-        sprintf(name, "%s.%s", csd, "string_encoding");
+        snprintf(name, sizeof(name), "%s.%s", csd, "string_encoding");
         _XlcGetResource(lcd, "XLC_CHARSET_DEFINE", name, &value, &num);
         if (num > 0) {
             _XlcDbg_printValue(name,value,num);
@@ -495,7 +495,7 @@ read_charset_define(
             }
         }
         /* sequence */
-        sprintf(name, "%s.%s", csd, "sequence");
+        snprintf(name, sizeof(name), "%s.%s", csd, "sequence");
         _XlcGetResource(lcd, "XLC_CHARSET_DEFINE", name, &value, &num);
         if (num > 0) {
             _XlcDbg_printValue(name,value,num);
@@ -511,7 +511,7 @@ read_charset_define(
             string_to_encoding(value[0],tmp);
         }
         /* encoding_name */
-        sprintf(name, "%s.%s", csd, "encoding_name");
+        snprintf(name, sizeof(name), "%s.%s", csd, "encoding_name");
         _XlcGetResource(lcd, "XLC_CHARSET_DEFINE", name, &value, &num);
         if (num > 0) {
             _XlcDbg_printValue(name,value,num);
@@ -565,10 +565,10 @@ read_segmentconversion(
     SegConv conversion;
     for (i=0 ; ; i++) { /* loop start */
         conversion = 0;
-        sprintf(conv, "conv%d", i);
+        snprintf(conv, sizeof(conv), "conv%d", i);
 
         /* length                */
-        sprintf(name, "%s.%s", conv, "length");
+        snprintf(name, sizeof(name), "%s.%s", conv, "length");
         _XlcGetResource(lcd, "XLC_SEGMENTCONVERSION", name, &value, &num);
         if (num > 0) {
             if (conversion == NULL &&
@@ -585,7 +585,7 @@ read_segmentconversion(
         conversion->length = atoi(value[0]);
 
         /* source_encoding       */
-        sprintf(name, "%s.%s", conv, "source_encoding");
+        snprintf(name, sizeof(name), "%s.%s", conv, "source_encoding");
         _XlcGetResource(lcd, "XLC_SEGMENTCONVERSION", name, &value, &num);
         if (num > 0) {
             char *tmp;
@@ -597,7 +597,7 @@ read_segmentconversion(
             conversion->source = srch_charset_define(tmp,&new);
         }
         /* destination_encoding  */
-        sprintf(name, "%s.%s", conv, "destination_encoding");
+        snprintf(name, sizeof(name), "%s.%s", conv, "destination_encoding");
         _XlcGetResource(lcd, "XLC_SEGMENTCONVERSION", name, &value, &num);
         if (num > 0) {
             char *tmp;
@@ -609,7 +609,7 @@ read_segmentconversion(
             conversion->dest = srch_charset_define(tmp,&new);
         }
         /* range                 */
-        sprintf(name, "%s.%s", conv, "range");
+        snprintf(name, sizeof(name), "%s.%s", conv, "range");
         _XlcGetResource(lcd, "XLC_SEGMENTCONVERSION", name, &value, &num);
         if (num > 0) {
             _XlcDbg_printValue(name,value,num);
@@ -617,7 +617,7 @@ read_segmentconversion(
                    &(conversion->range.start), &(conversion->range.end));
         }
         /* conversion            */
-        sprintf(name, "%s.%s", conv, "conversion");
+        snprintf(name, sizeof(name), "%s.%s", conv, "conversion");
         _XlcGetResource(lcd, "XLC_SEGMENTCONVERSION", name, &value, &num);
         if (num > 0) {
             _XlcDbg_printValue(name,value,num);
@@ -635,6 +635,7 @@ create_ctextseg(
     ExtdSegment ret;
     char* ptr;
     char* cset_name = NULL;
+    size_t cset_len;
     int i,new;
     FontScope scope;
     ret = (ExtdSegment)Xmalloc(sizeof(ExtdSegmentRec));
@@ -645,7 +646,8 @@ create_ctextseg(
         Xfree (ret);
         return NULL;
     }
-    cset_name = (char*) Xmalloc (strlen(ret->name) + 1);
+    cset_len = strlen(ret->name) + 1;
+    cset_name = Xmalloc (cset_len);
     if (cset_name == NULL) {
         Xfree (ret->name);
         Xfree (ret);
@@ -657,10 +659,10 @@ create_ctextseg(
         ptr++;
         if (!_XlcNCompareISOLatin1(ptr, "GL", 2)) {
             ret->side =  XlcGL;
-            sprintf(cset_name,"%s:%s",ret->name,"GL");
+            snprintf(cset_name, cset_len, "%s:%s", ret->name, "GL");
         } else {
             ret->side =  XlcGR;
-            sprintf(cset_name,"%s:%s",ret->name,"GR");
+            snprintf(cset_name, cset_len, "%s:%s", ret->name, "GR");
         }
     } else {
         ret->side =  XlcGLGR;
@@ -731,10 +733,10 @@ load_generic(
 	char cs[16];
 	char name[BUFSIZ];
 
-	sprintf(cs, "cs%d", i);
+	snprintf(cs, sizeof(cs), "cs%d", i);
 
 	/***** codeset.side *****/
-	sprintf(name, "%s.%s", cs , "side");
+	snprintf(name, sizeof(name), "%s.%s", cs , "side");
 	_XlcGetResource(lcd, "XLC_XLOCALE", name, &value, &num);
 	if (num > 0) {
 	    char *tmp;
@@ -761,7 +763,7 @@ load_generic(
 	}
 
 	/***** codeset.length *****/
-	sprintf(name, "%s.%s", cs , "length");
+	snprintf(name, sizeof(name), "%s.%s", cs , "length");
 	_XlcGetResource(lcd, "XLC_XLOCALE", name, &value, &num);
 	if (num > 0) {
 	    if (codeset == NULL && (codeset = add_codeset(gen)) == NULL)
@@ -772,7 +774,7 @@ load_generic(
 	}
 
 	/***** codeset.mb_encoding *****/
-	sprintf(name, "%s.%s", cs, "mb_encoding");
+	snprintf(name, sizeof(name), "%s.%s", cs, "mb_encoding");
 	_XlcGetResource(lcd, "XLC_XLOCALE", name, &value, &num);
 	if (num > 0) {
 	    static struct {
@@ -808,7 +810,7 @@ load_generic(
 	}
 
 	/***** codeset.wc_encoding *****/
-	sprintf(name, "%s.%s", cs, "wc_encoding");
+	snprintf(name, sizeof(name), "%s.%s", cs, "wc_encoding");
 	_XlcGetResource(lcd, "XLC_XLOCALE", name, &value, &num);
 	if (num > 0) {
 	    if (codeset == NULL && (codeset = add_codeset(gen)) == NULL)
@@ -819,7 +821,7 @@ load_generic(
 	}
 
 	/***** codeset.ct_encoding *****/
-	sprintf(name, "%s.%s", cs, "ct_encoding");
+	snprintf(name, sizeof(name), "%s.%s", cs, "ct_encoding");
 	_XlcGetResource(lcd, "XLC_XLOCALE", name, &value, &num);
 	if (num > 0) {
 	    char *encoding;
@@ -861,7 +863,7 @@ load_generic(
             unsigned long start,end;
             ByteInfo tmpb;
 
-            sprintf(name,"%s.%s%d",cs,"byte",M);
+            snprintf(name, sizeof(name),"%s.%s%d",cs,"byte",M);
             _XlcGetResource(lcd, "XLC_XLOCALE", name, &value, &num);
 
             if (M == 1) {
@@ -896,7 +898,7 @@ load_generic(
 
 
         /***** codeset.mb_conversion *****/
-        sprintf(name, "%s.%s", cs, "mb_conversion");
+        snprintf(name, sizeof(name), "%s.%s", cs, "mb_conversion");
         _XlcGetResource(lcd, "XLC_XLOCALE", name, &value, &num);
         if (num > 0) {
                 _XlcDbg_printValue(name,value,num);
@@ -908,7 +910,7 @@ load_generic(
                 /* [\x%x,\x%x]->\x%x,... */
         }
         /***** codeset.ct_conversion *****/
-        sprintf(name, "%s.%s", cs, "ct_conversion");
+        snprintf(name, sizeof(name), "%s.%s", cs, "ct_conversion");
         _XlcGetResource(lcd, "XLC_XLOCALE", name, &value, &num);
         if (num > 0) {
                 _XlcDbg_printValue(name,value,num);
@@ -920,14 +922,14 @@ load_generic(
                 /* [\x%x,\x%x]->\x%x,... */
         }
         /***** codeset.ct_conversion_file *****/
-        sprintf(name, "%s.%s", cs, "ct_conversion_file");
+        snprintf(name, sizeof(name), "%s.%s", cs, "ct_conversion_file");
         _XlcGetResource(lcd, "XLC_XLOCALE", name, &value, &num);
         if (num > 0) {
                 _XlcDbg_printValue(name,value,num);
                 /* [\x%x,\x%x]->\x%x,... */
         }
         /***** codeset.ct_extended_segment *****/
-        sprintf(name, "%s.%s", cs, "ct_extended_segment");
+        snprintf(name, sizeof(name), "%s.%s", cs, "ct_extended_segment");
         _XlcGetResource(lcd, "XLC_XLOCALE", name, &value, &num);
         if (num > 0) {
                 _XlcDbg_printValue(name,value,num);
diff --git a/src/xlibi18n/lcUTF8.c b/src/xlibi18n/lcUTF8.c
index 3e934b7..84d8777 100644
--- a/src/xlibi18n/lcUTF8.c
+++ b/src/xlibi18n/lcUTF8.c
@@ -1731,10 +1731,10 @@ create_tofontcs_conv(
     lazy_init_all_charsets();
 
     for (i = 0, num = 0;; i++) {
-	sprintf(buf, "fs%d.charset.name", i);
+	snprintf(buf, sizeof(buf), "fs%d.charset.name", i);
 	_XlcGetResource(lcd, "XLC_FONTSET", buf, &value, &count);
 	if (count < 1) {
-	    sprintf(buf, "fs%d.charset", i);
+	    snprintf(buf, sizeof(buf), "fs%d.charset", i);
 	    _XlcGetResource(lcd, "XLC_FONTSET", buf, &value, &count);
 	    if (count < 1)
 		break;
@@ -1749,10 +1749,10 @@ create_tofontcs_conv(
 
     /* Loop through all fontsets mentioned in the locale. */
     for (i = 0, num = 0;; i++) {
-        sprintf(buf, "fs%d.charset.name", i);
+        snprintf(buf, sizeof(buf), "fs%d.charset.name", i);
         _XlcGetResource(lcd, "XLC_FONTSET", buf, &value, &count);
         if (count < 1) {
-            sprintf(buf, "fs%d.charset", i);
+            snprintf(buf, sizeof(buf), "fs%d.charset", i);
             _XlcGetResource(lcd, "XLC_FONTSET", buf, &value, &count);
             if (count < 1)
                 break;
-- 
1.7.9.2



More information about the xorg-devel mailing list