Clean up code for widget_in() and widget_out().
authorTom Lane <tgl@sss.pgh.pa.us>
Sat, 9 Jan 2016 18:44:27 +0000 (13:44 -0500)
committerTom Lane <tgl@sss.pgh.pa.us>
Sat, 9 Jan 2016 18:44:27 +0000 (13:44 -0500)
Given syntactically wrong input, widget_in() could call atof() with an
indeterminate pointer argument, typically leading to a crash; or if it
didn't do that, it might return a NULL pointer, which again would lead
to a crash since old-style C functions aren't supposed to do things
that way.  Fix that by correcting the off-by-one syntax test and
throwing a proper error rather than just returning NULL.

Also, since widget_in and widget_out have been marked STRICT for a
long time, their tests for null inputs are just dead code; remove 'em.
In the oldest branches, also improve widget_out to use snprintf not
sprintf, just to be sure.

In passing, get rid of a long-since-useless sprintf into a local buffer
that nothing further is done with, and make some other minor coding
style cleanups.

In the intended regression-testing usage of these functions, none of
this is very significant; but if the regression test database were
left around in a production installation, these bugs could amount
to a minor security hazard.

Piotr Stefaniak, Michael Paquier, and Tom Lane

src/test/regress/regress.c

index be76f7dc416deb30220c876f5e22a06b897cb6f7..697b39aae3a80a822277ce122fcf5b56a3fff314 100644 (file)
@@ -243,25 +243,27 @@ WIDGET *
 widget_in(char *str)
 {
        char       *p,
-                          *coord[NARGS],
-                               buf2[1000];
+                          *coord[NARGS];
        int                     i;
        WIDGET     *result;
 
-       if (str == NULL)
-               return NULL;
        for (i = 0, p = str; *p && i < NARGS && *p != RDELIM; p++)
-               if (*p == ',' || (*p == LDELIM && !i))
+       {
+               if (*p == DELIM || (*p == LDELIM && i == 0))
                        coord[i++] = p + 1;
-       if (i < NARGS - 1)
-               return NULL;
+       }
+
+       if (i < NARGS)
+               ereport(ERROR,
+                               (errcode(ERRCODE_INVALID_TEXT_REPRESENTATION),
+                                errmsg("invalid input syntax for type widget: \"%s\"",
+                                               str)));
+
        result = (WIDGET *) palloc(sizeof(WIDGET));
        result->center.x = atof(coord[0]);
        result->center.y = atof(coord[1]);
        result->radius = atof(coord[2]);
 
-       snprintf(buf2, sizeof(buf2), "widget_in: read (%f, %f, %f)\n",
-                        result->center.x, result->center.y, result->radius);
        return result;
 }
 
@@ -270,12 +272,9 @@ widget_out(WIDGET * widget)
 {
        char       *result;
 
-       if (widget == NULL)
-               return NULL;
-
-       result = (char *) palloc(60);
-       sprintf(result, "(%g,%g,%g)",
-                       widget->center.x, widget->center.y, widget->radius);
+       result = (char *) palloc(100);
+       snprintf(result, 100, "(%g,%g,%g)",
+                        widget->center.x, widget->center.y, widget->radius);
        return result;
 }