[PATCH:libX11 02/12] xlibi18n: convert sprintf calls to snprintf
Matthieu Herrb
matthieu.herrb at laas.fr
Sun Aug 11 02:46:35 PDT 2013
On Sat, Aug 10, 2013 at 01:54:59PM -0700, Alan Coopersmith wrote:
> 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>
The first part doen't match the description above, but makes sense
too. Perhaps commit it separatly ?
Also, given how args[] is built in parse_line() I fail to see how one
element could be NULL.
but a bit of extra paranoia can't hurt, and for all the sprintf ->
snprintf conversions:
Reviewed-by: Matthieu Herrb <matthieu.herrb at laas.fr>
> ---
> 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
>
> _______________________________________________
> xorg-devel at lists.x.org: X.Org development
> Archives: http://lists.x.org/archives/xorg-devel
> Info: http://lists.x.org/mailman/listinfo/xorg-devel
--
Matthieu Herrb
More information about the xorg-devel
mailing list