Skip to content

Commit

Permalink
sql: check printf() for failure
Browse files Browse the repository at this point in the history
This patch adds a check that sqlXPrintf() does not fail in the built-in
SQL function printf(). There are two possible problems: the result might
get too large, or there might be an integer overflow because internally
int values are converted to size_t.

Closes #tarantool/security#122

NO_DOC=bugfix
  • Loading branch information
ImeevMA authored and igormunkin committed May 23, 2023
1 parent 039f714 commit 1315923
Show file tree
Hide file tree
Showing 5 changed files with 42 additions and 3 deletions.
4 changes: 4 additions & 0 deletions changelogs/unreleased/ghs-122-allocations-in-printf.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
## bugfix/sql

* Fixed an integer overflow issue and added check for the `printf()` failure due
to too large size (ghs-122).
6 changes: 6 additions & 0 deletions src/box/sql/func.c
Original file line number Diff line number Diff line change
Expand Up @@ -923,6 +923,12 @@ func_printf(struct sql_context *ctx, int argc, const struct Mem *argv)
sqlStrAccumInit(&acc, NULL, 0, SQL_MAX_LENGTH);
acc.printfFlags = SQL_PRINTF_SQLFUNC;
sqlXPrintf(&acc, format, &pargs);
assert(acc.accError == 0 || acc.accError == STRACCUM_TOOBIG);
if (acc.accError == STRACCUM_TOOBIG) {
ctx->is_aborted = true;
diag_set(ClientError, ER_SQL_EXECUTE, "string or blob too big");
return;
}
mem_set_str_allocated(ctx->pOut, sqlStrAccumFinish(&acc), acc.nChar);
}

Expand Down
6 changes: 4 additions & 2 deletions src/box/sql/printf.c
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,8 @@ sqlVXPrintf(StrAccum * pAccum, /* Accumulate results here */
{
int c; /* Next character in the format string */
char *bufpt; /* Pointer to the conversion buffer */
int precision; /* Precision of the current field */
/* Precision of the current field. */
int64_t precision;
int length; /* Length of the field */
int idx; /* A general purpose loop counter */
int width; /* Width of the current field */
Expand All @@ -204,7 +205,8 @@ sqlVXPrintf(StrAccum * pAccum, /* Accumulate results here */
LONGDOUBLE_TYPE realvalue; /* Value for real types */
const et_info *infop; /* Pointer to the appropriate info structure */
char *zOut; /* Rendering buffer */
int nOut; /* Size of the rendering buffer */
/* Size of the rendering buffer. */
size_t nOut;
char *zExtra = 0; /* Malloced memory used by some conversion */
int exp, e2; /* exponent of real numbers */
int nsd; /* Number of significant digits returned */
Expand Down
27 changes: 27 additions & 0 deletions test/sql-luatest/ghs_122_allocations_in_printf_test.lua
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
local server = require('luatest.server')
local t = require('luatest')

local g = t.group()

g.before_all(function()
g.server = server:new({alias = 'master'})
g.server:start()
end)

g.after_all(function()
g.server:stop()
end)

g.test_printf = function()
g.server:exec(function()
local msg = [[Failed to execute SQL statement: string or blob too big]]

local ret, err = box.execute([[SELECT printf('%.*d', 0x7ffffff0, 0);]])
t.assert(ret == nil)
t.assert_equals(err.message, msg)

ret, err = box.execute("SELECT printf('hello %.*d', 0x7fffffff, 0);")
t.assert(ret == nil)
t.assert_equals(err.message, msg)
end)
end
2 changes: 1 addition & 1 deletion test/sql-luatest/suite.ini
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
[default]
core = luatest
description = SQL tests on luatest
long_run = sql-luatest/ghs_119_too_long_mem_values_test.lua
long_run = sql-luatest/ghs_119_too_long_mem_values_test.lua sql-luatest/ghs_122_allocations_in_printf_test.lua

0 comments on commit 1315923

Please sign in to comment.