GNOME.org
 

[babl] babl: use snprintf instead of sprintf

commit 2782d926057c49ac2321d1a34d579da4b3ead6b2
Author: Tobias Stoeckmann <tobias stoeckmann org>
Date:   Tue Oct 24 20:02:15 2017 +0200

    babl: use snprintf instead of sprintf
    
    Using sprintf with environment variables is dangerous, because it can
    easily lead to out of boundary writes on heap space.
    
    While at it, replace sprintf calls with snprintf where proper
    boundary checks are possible and required.
    
    Signed-off-by: Tobias Stoeckmann <tobias stoeckmann org>

 babl/babl-cache.c      |   18 +++++++++---------
 babl/babl-conversion.c |    6 ++----
 babl/babl-format.c     |   20 +++++++++++++-------
 babl/babl-icc.c        |    2 +-
 babl/babl-internal.c   |    2 +-
 babl/babl-memory.c     |    2 +-
 babl/babl-palette.c    |    2 +-
 babl/babl-space.c      |    9 +++++----
 babl/babl-trc.c        |   10 +++++-----
 9 files changed, 38 insertions(+), 33 deletions(-)
---
diff --git a/babl/babl-cache.c b/babl/babl-cache.c
index 4954e4b..13badc5 100644
--- a/babl/babl-cache.c
+++ b/babl/babl-cache.c
@@ -82,16 +82,16 @@ static const char *fish_cache_path (void)
   path[sizeof (path) - 1] = '\0';
 #ifndef _WIN32
   if (getenv ("XDG_CACHE_HOME"))
-    sprintf (path, "%s/babl/babl-fishes", getenv("XDG_CACHE_HOME"));
+    snprintf (path, sizeof (path), "%s/babl/babl-fishes", getenv("XDG_CACHE_HOME"));
   else if (getenv ("HOME"))
-    sprintf (path, "%s/.cache/babl/babl-fishes", getenv("HOME"));
+    snprintf (path, sizeof (path), "%s/.cache/babl/babl-fishes", getenv("HOME"));
 #else
 {
   char win32path[4096];
   if (SHGetFolderPathA (NULL, CSIDL_LOCAL_APPDATA, NULL, SHGFP_TYPE_CURRENT, win32path) == S_OK)
-    sprintf (path, "%s\\%s\\babl-fishes.txt", win32path, BABL_LIBRARY);
+    snprintf (path, sizeof (path), "%s\\%s\\babl-fishes.txt", win32path, BABL_LIBRARY);
   else if (getenv ("TEMP"))
-    sprintf (path, "%s\\babl-fishes.txt", getenv("TEMP"));
+    snprintf (path, sizeof (path), "%s\\babl-fishes.txt", getenv("TEMP"));
 }
 #endif
 
@@ -153,13 +153,13 @@ static const char *cache_header (void)
 {
   static char buf[2048];
   if (strchr (BABL_GIT_VERSION, ' ')) // we must be building from tarball
-    sprintf (buf, "#%i.%i.%i BABL_PATH_LENGTH=%d BABL_TOLERANCE=%f",
+    snprintf (buf, sizeof (buf),
+             "#%i.%i.%i BABL_PATH_LENGTH=%d BABL_TOLERANCE=%f",
              BABL_MAJOR_VERSION, BABL_MINOR_VERSION, BABL_MICRO_VERSION,
              _babl_max_path_len (), _babl_legal_error ());
   else
-    sprintf (buf, "#%s BABL_PATH_LENGTH=%d BABL_TOLERANCE=%f",
-             BABL_GIT_VERSION,
-             _babl_max_path_len (), _babl_legal_error ());
+    snprintf (buf, sizeof (buf), "#%s BABL_PATH_LENGTH=%d BABL_TOLERANCE=%f",
+             BABL_GIT_VERSION, _babl_max_path_len (), _babl_legal_error ());
   return buf;
 }
 
@@ -170,7 +170,7 @@ void babl_store_db (void)
   char *tmpp = calloc(8000,1);
   FILE *dbfile;
 
-  sprintf (tmpp, "%s~", fish_cache_path ());
+  snprintf (tmpp, 8000, "%s~", fish_cache_path ());
   dbfile  = fopen (tmpp, "w");
   if (!dbfile)
     return;
diff --git a/babl/babl-conversion.c b/babl/babl-conversion.c
index 75e5c93..17dccf8 100644
--- a/babl/babl-conversion.c
+++ b/babl/babl-conversion.c
@@ -154,7 +154,7 @@ create_name (Babl *source, Babl *destination, int type)
 {
   if (babl_extender ())
     {
-      snprintf (buf, 512 - 1, "%s %i: %s%s to %s",
+      snprintf (buf, sizeof (buf), "%s %i: %s%s to %s",
                 BABL (babl_extender ())->instance.name,
                 collisions,
                 type == BABL_CONVERSION_LINEAR ? "" :
@@ -162,18 +162,16 @@ create_name (Babl *source, Babl *destination, int type)
                 type == BABL_CONVERSION_PLANAR ? "planar " : "Eeeek! ",
                 source->instance.name,
                 destination->instance.name);
-      buf[511] = '\0';
     }
   else
     {
-      snprintf (buf, 512 - 1, "%s %s to %s %i",
+      snprintf (buf, sizeof (buf), "%s %s to %s %i",
                 type == BABL_CONVERSION_LINEAR ? "" :
                 type == BABL_CONVERSION_PLANE ? "plane " :
                 type == BABL_CONVERSION_PLANAR ? "planar " : "Eeeek! ",
                 source->instance.name,
                 destination->instance.name,
                 collisions);
-      buf[511] = '\0';
     }
   return buf;
 }
diff --git a/babl/babl-format.c b/babl/babl-format.c
index 4ed14d0..079272c 100644
--- a/babl/babl-format.c
+++ b/babl/babl-format.c
@@ -135,8 +135,8 @@ format_new_from_format_with_space (const Babl *format, const Babl *space)
 {
   Babl *ret;
   char new_name[256];
-  sprintf (new_name, "%s-%s", babl_get_name ((void*)format),
-                              babl_get_name ((void*)space));
+  snprintf (new_name, sizeof (new_name), "%s-%s", babl_get_name ((void*)format),
+                                                  babl_get_name ((void*)space));
   ret = babl_db_find (babl_format_db(), new_name);
   if (ret)
     return ret;
@@ -161,6 +161,7 @@ create_name (const BablModel *model,
 {
   char            buf[512] = "";
   char           *p = &buf[0];
+  ssize_t         left;
   int             i;
   int             same_types = 1;
   const BablType**t          = type;
@@ -168,9 +169,11 @@ create_name (const BablModel *model,
   BablComponent **c1         = component;
   BablComponent **c2         = model->component;
 
-
-  sprintf (p, "%s ", model->instance.name);
+  left = 512;
+  snprintf (p, left, "%s ", model->instance.name);
   p += strlen (model->instance.name) + 1;
+  left -= strlen (model->instance.name) + 1;
+  babl_assert (left >= 0);
 
   i = components;
   while (i--)
@@ -202,7 +205,7 @@ create_name (const BablModel *model,
 
   if (same_types)
     {
-      sprintf (p, "%s", first_type->instance.name);
+      snprintf (p, left, "%s", first_type->instance.name);
       return babl_strdup (buf);
     }
 
@@ -210,11 +213,14 @@ create_name (const BablModel *model,
 
   while (i--)
     {
-      sprintf (p, "(%s as %s) ",
+      snprintf (p, left, "(%s as %s) ",
                (*component)->instance.name,
                (*type)->instance.name);
       p += strlen ((*component)->instance.name) +
            strlen ((*type)->instance.name) + strlen ("( as ) ");
+      left -= strlen ((*component)->instance.name) +
+              strlen ((*type)->instance.name) + strlen ("( as ) ");
+      babl_assert (left >= 0);
       component++;
       type++;
     }
@@ -226,7 +232,7 @@ ncomponents_create_name (const Babl *type,
                          int         components)
 {
   char buf[512];
-  sprintf (buf, "%s[%i] ", type->instance.name, components);
+  snprintf (buf, sizeof (buf), "%s[%i] ", type->instance.name, components);
   return babl_strdup (buf);
 }
 
diff --git a/babl/babl-icc.c b/babl/babl-icc.c
index 45ba8fa..20841b9 100644
--- a/babl/babl-icc.c
+++ b/babl/babl-icc.c
@@ -973,7 +973,7 @@ char *babl_icc_get_key (const char *icc_data,
   {
     char tag[5];
     int val = icc_read (u32, 64);
-    sprintf (tag, "%i", val);
+    snprintf (tag, sizeof (tag), "%i", val);
     return strdup (tag);
   } else if (!strcmp (key, "tags"))
   {
diff --git a/babl/babl-internal.c b/babl/babl-internal.c
index 0589e3f..e0b4f7a 100644
--- a/babl/babl-internal.c
+++ b/babl/babl-internal.c
@@ -65,7 +65,7 @@ babl_backtrack (void)
 {
   char buf[512];
 
-  sprintf (buf, "echo bt>/tmp/babl.gdb;"
+  snprintf (buf, sizeof (buf), "echo bt>/tmp/babl.gdb;"
            "gdb -q --batch -x /tmp/babl.gdb --pid=%i | grep 'in ''babl_die' -A40", getpid ());
   return system (buf);
 }
diff --git a/babl/babl-memory.c b/babl/babl-memory.c
index d9e4d2f..d0229e4 100644
--- a/babl/babl-memory.c
+++ b/babl/babl-memory.c
@@ -71,7 +71,7 @@ mem_stats (void)
 {
   static char buf[128];
 
-  sprintf (buf, "mallocs:%i callocs:%i strdups:%i dups:%i allocs:%i frees:%i reallocs:%i\t|",
+  snprintf (buf, sizeof (buf), "mallocs:%i callocs:%i strdups:%i dups:%i allocs:%i frees:%i reallocs:%i\t|",
            mallocs, callocs, strdups, dups, mallocs + callocs + strdups + dups, frees, reallocs);
   return buf;
 }
diff --git a/babl/babl-palette.c b/babl/babl-palette.c
index 823ff0c..fda96ae 100644
--- a/babl/babl-palette.c
+++ b/babl/babl-palette.c
@@ -483,7 +483,7 @@ const Babl *babl_new_palette (const char  *name,
   if (!name)
     {
       static int cnt = 0;
-      sprintf (cname, "_babl-int-%i", cnt++);
+      snprintf (cname, sizeof (cname), "_babl-int-%i", cnt++);
       name = cname;
     }
   else
diff --git a/babl/babl-space.c b/babl/babl-space.c
index adc9e7a..781e419 100644
--- a/babl/babl-space.c
+++ b/babl/babl-space.c
@@ -285,9 +285,9 @@ babl_space_from_rgbxyz_matrix (const char *name,
   space_db[i]=space;
   space_db[i].instance.name = space_db[i].name;
   if (name)
-    sprintf (space_db[i].name, "%s", name);
+    snprintf (space_db[i].name, sizeof (space_db[i].name), "%s", name);
   else
-    sprintf (space_db[i].name, "space-%.4f,%.4f_%.4f,%.4f_%.4f_%.4f,%.4f_%.4f,%.4f_%s,%s,%s",
+    snprintf (space_db[i].name, sizeof (space_db[i].name), 
"space-%.4f,%.4f_%.4f,%.4f_%.4f_%.4f,%.4f_%.4f,%.4f_%s,%s,%s",
                        rx, gx, bx,
                        ry, gy, by,
                        rz, gz, bz,
@@ -348,10 +348,11 @@ babl_space_from_chromaticities (const char *name,
   space_db[i]=space;
   space_db[i].instance.name = space_db[i].name;
   if (name)
-    sprintf (space_db[i].name, "%s", name);
+    snprintf (space_db[i].name, sizeof (space_db[i].name), "%s", name);
   else
           /* XXX: this can get longer than 256bytes ! */
-    sprintf (space_db[i].name, "space-%.4f,%.4f_%.4f,%.4f_%.4f,%.4f_%.4f,%.4f_%s,%s,%s",
+    snprintf (space_db[i].name, sizeof (space_db[i].name),
+             "space-%.4f,%.4f_%.4f,%.4f_%.4f,%.4f_%.4f,%.4f_%s,%s,%s",
              wx,wy,rx,ry,bx,by,gx,gy,babl_get_name (space.trc[0]),
              babl_get_name(space.trc[1]), babl_get_name(space.trc[2]));
 
diff --git a/babl/babl-trc.c b/babl/babl-trc.c
index 7524ef8..0a00710 100644
--- a/babl/babl-trc.c
+++ b/babl/babl-trc.c
@@ -292,11 +292,11 @@ babl_trc_new (const char *name,
   trc_db[i]=trc;
   trc_db[i].instance.name = trc_db[i].name;
   if (name)
-    sprintf (trc_db[i].name, "%s", name);
+    snprintf (trc_db[i].name, sizeof (trc_db[i].name), "%s", name);
   else if (n_lut)
-    sprintf (trc_db[i].name, "lut-trc");
+    snprintf (trc_db[i].name, sizeof (trc_db[i].name), "lut-trc");
   else
-    sprintf (trc_db[i].name, "trc-%i-%f", type, gamma);
+    snprintf (trc_db[i].name, sizeof (trc_db[i].name), "trc-%i-%f", type, gamma);
 
   if (n_lut)
   {
@@ -430,7 +430,7 @@ babl_trc_formula_srgb (double g, double a, double b, double c, double d)
       fabs (c - (-3417))  < 0.01)
     return babl_trc ("sRGB");
 
-  sprintf (name, "%.6f %.6f %.4f %.4f %.4f", g, a, b, c, d);
+  snprintf (name, sizeof (name), "%.6f %.6f %.4f %.4f %.4f", g, a, b, c, d);
   for (i = 0; name[i]; i++)
     if (name[i] == ',') name[i] = '.';
   while (name[strlen(name)-1]=='0')
@@ -446,7 +446,7 @@ babl_trc_gamma (double gamma)
   if (fabs (gamma - 1.0) < 0.01)
      return babl_trc_new ("linear", BABL_TRC_LINEAR, 1.0, 0, NULL);
 
-  sprintf (name, "%.6f", gamma);
+  snprintf (name, sizeof (name), "%.6f", gamma);
   for (i = 0; name[i]; i++)
     if (name[i] == ',') name[i] = '.';
   while (name[strlen(name)-1]=='0')
[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]