From fe2578568e769d322036953cb77acea677aaac6a Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Sat, 9 Jan 2016 13:44:27 -0500 Subject: [PATCH] Clean up code for widget_in() and widget_out(). 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 | 29 ++++++++++++++--------------- 1 file changed, 14 insertions(+), 15 deletions(-) diff --git a/src/test/regress/regress.c b/src/test/regress/regress.c index be76f7dc41..697b39aae3 100644 --- a/src/test/regress/regress.c +++ b/src/test/regress/regress.c @@ -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; } -- 2.40.0