[tarantool-patches] [PATCH] sql: do not allow oversized integer literals

  • From: Kirill Yukhin <kyukhin@xxxxxxxxxxxxx>
  • To: korablev@xxxxxxxxxxxxx
  • Date: Tue, 17 Jul 2018 09:08:41 +0300

Before the patch, big integer constant were silently
converted to floating point values. Fix that by issuing
error message if value doesn't fit in int64_t range.

Also, refactor surrounding code as per Tarantool's code style.

Closes #2347
---
Issue: https://github.com/tarantool/tarantool/issues/2347
Branch: 
https://github.com/tarantool/tarantool/commits/kyukhin/gh-2347-report-error-on-int-overflow

 src/box/sql/expr.c                         | 70 +++++++++++----------
 src/box/sql/main.c                         |  5 +-
 src/box/sql/sqliteInt.h                    | 49 ++++++++++++++-
 src/box/sql/util.c                         | 98 +++++++++++++-----------------
 src/box/sql/vdbe.c                         |  8 +--
 src/box/sql/vdbemem.c                      |  6 +-
 test/sql-tap/default.test.lua              |  5 +-
 test/sql/gh-2347-max-int-literals.test.lua | 11 ++++
 8 files changed, 144 insertions(+), 108 deletions(-)
 create mode 100644 test/sql/gh-2347-max-int-literals.test.lua

diff --git a/src/box/sql/expr.c b/src/box/sql/expr.c
index b1650cf..43d71eb 100644
--- a/src/box/sql/expr.c
+++ b/src/box/sql/expr.c
@@ -1087,9 +1087,9 @@ sqlite3ExprAssignVarNumber(Parse * pParse, Expr * pExpr, 
u32 n)
                         * "nnn" to an integer and use it as the
                         * variable number
                         */
-                       i64 i;
+                       int64_t i;
                        int bOk =
-                           0 == sqlite3Atoi64(&z[1], &i, n - 1);
+                           0 == sql_atoi64(&z[1], &i, n - 1);
                        x = (ynVar) i;
                        testcase(i == 0);
                        testcase(i == 1);
@@ -3259,51 +3259,49 @@ codeReal(Vdbe * v, const char *z, int negateFlag, int 
iMem)
 }
 #endif
 
-/*
+/**
  * Generate an instruction that will put the integer describe by
  * text z[0..n-1] into register iMem.
  *
- * Expr.u.zToken is always UTF8 and zero-terminated.
+ * @param parse Parsing context.
+ * @param expr Expression being parsed. Expr.u.zToken is always
+ *             UTF8 and zero-terminated.
+ * @param neg_flag True if value is negative.
+ * @param mem Register to store parsed integer
  */
 static void
-codeInteger(Parse * pParse, Expr * pExpr, int negFlag, int iMem)
+sql_expr_code_int(struct Parse *parse, struct Expr *expr, bool neg_flag,
+                 int mem)
 {
-       Vdbe *v = pParse->pVdbe;
-       if (pExpr->flags & EP_IntValue) {
-               int i = pExpr->u.iValue;
+       struct Vdbe *v = parse->pVdbe;
+       if (expr->flags & EP_IntValue) {
+               int i = expr->u.iValue;
                assert(i >= 0);
-               if (negFlag)
+               if (neg_flag)
                        i = -i;
-               sqlite3VdbeAddOp2(v, OP_Integer, i, iMem);
+               sqlite3VdbeAddOp2(v, OP_Integer, i, mem);
        } else {
                int c;
-               i64 value;
-               const char *z = pExpr->u.zToken;
-               assert(z != 0);
-               c = sqlite3DecOrHexToI64(z, &value);
-               if (c == 1 || (c == 2 && !negFlag)
-                   || (negFlag && value == SMALLEST_INT64)) {
-#ifdef SQLITE_OMIT_FLOATING_POINT
-                       sqlite3ErrorMsg(pParse, "oversized integer: %s%s",
-                                       negFlag ? "-" : "", z);
-#else
-#ifndef SQLITE_OMIT_HEX_INTEGER
-                       if (sqlite3_strnicmp(z, "0x", 2) == 0) {
-                               sqlite3ErrorMsg(pParse,
-                                               "hex literal too big: %s%s",
-                                               negFlag ? "-" : "", z);
-                       } else
-#endif
-                       {
-                               codeReal(v, z, negFlag, iMem);
+               int64_t value;
+               const char *z = expr->u.zToken;
+               assert(z != NULL);
+               c = sql_dec_or_hex_to_i64(z, &value);
+               if (c == 1 || (c == 2 && !neg_flag)
+                   || (neg_flag && value == SMALLEST_INT64)) {
+                        if (sqlite3_strnicmp(z, "0x", 2) == 0) {
+                                sqlite3ErrorMsg(parse,
+                                                "hex literal too big: %s%s",
+                                                neg_flag ? "-" : "", z);
+                       } else {
+                               sqlite3ErrorMsg(parse,
+                                               "oversized integer: %s%s",
+                                               neg_flag ? "-" : "", z);
                        }
-#endif
                } else {
-                       if (negFlag) {
+                       if (neg_flag)
                                value = c == 2 ? SMALLEST_INT64 : -value;
-                       }
-                       sqlite3VdbeAddOp4Dup8(v, OP_Int64, 0, iMem, 0,
-                                             (u8 *) & value, P4_INT64);
+                       sqlite3VdbeAddOp4Dup8(v, OP_Int64, 0, mem, 0,
+                                             (u8 *)&value, P4_INT64);
                }
        }
 }
@@ -3715,7 +3713,7 @@ sqlite3ExprCodeTarget(Parse * pParse, Expr * pExpr, int 
target)
                                                        target, pExpr->op2);
                }
        case TK_INTEGER:{
-                       codeInteger(pParse, pExpr, 0, target);
+                       sql_expr_code_int(pParse, pExpr, false, target);
                        return target;
                }
 #ifndef SQLITE_OMIT_FLOATING_POINT
@@ -3875,7 +3873,7 @@ sqlite3ExprCodeTarget(Parse * pParse, Expr * pExpr, int 
target)
                        Expr *pLeft = pExpr->pLeft;
                        assert(pLeft);
                        if (pLeft->op == TK_INTEGER) {
-                               codeInteger(pParse, pLeft, 1, target);
+                               sql_expr_code_int(pParse, pLeft, true, target);
                                return target;
 #ifndef SQLITE_OMIT_FLOATING_POINT
                        } else if (pLeft->op == TK_FLOAT) {
diff --git a/src/box/sql/main.c b/src/box/sql/main.c
index 00dc7a6..cbca90a 100644
--- a/src/box/sql/main.c
+++ b/src/box/sql/main.c
@@ -2468,10 +2468,9 @@ sqlite3_uri_int64(const char *zFilename, /* Filename as 
passed to xOpen */
                  sqlite3_int64 bDflt)  /* return if parameter is missing */
 {
        const char *z = sqlite3_uri_parameter(zFilename, zParam);
-       sqlite3_int64 v;
-       if (z && sqlite3DecOrHexToI64(z, &v) == SQLITE_OK) {
+       int64_t v;
+       if (z != NULL && sql_dec_or_hex_to_i64(z, &v) == SQLITE_OK)
                bDflt = v;
-       }
        return bDflt;
 }
 
diff --git a/src/box/sql/sqliteInt.h b/src/box/sql/sqliteInt.h
index 18bf949..1748607 100644
--- a/src/box/sql/sqliteInt.h
+++ b/src/box/sql/sqliteInt.h
@@ -4354,8 +4354,53 @@ char
 sqlite3TableColumnAffinity(struct space_def *def, int idx);
 
 char sqlite3ExprAffinity(Expr * pExpr);
-int sqlite3Atoi64(const char *, i64 *, int);
-int sqlite3DecOrHexToI64(const char *, i64 *);
+
+
+/**
+ * Convert z to a 64-bit signed integer.  z must be decimal. This
+ * routine does *not* accept hexadecimal notation.
+ *
+ * If the z value is representable as a 64-bit twos-complement
+ * integer, then write that value into *val and return 0.
+ *
+ * If z is exactly 9223372036854775808, return 2.  This special
+ * case is broken out because while 9223372036854775808 cannot be a
+ * signed 64-bit integer, its negative -9223372036854775808 can be.
+ *
+ * If z is too big for a 64-bit integer and is not
+ * 9223372036854775808  or if z contains any non-numeric text,
+ * then return 1.
+ *
+ * length is the number of bytes in the string (bytes, not characters).
+ * The string is not necessarily zero-terminated.  The encoding is
+ * given by enc.
+ *
+ * @param z String being parsed.
+ * @param[out] val Output integer value.
+ * @param length String length in bytes.
+ * @retval
+ *     0    Successful transformation.  Fits in a 64-bit signed integer.
+ *     1    Integer too large for a 64-bit signed integer or is malformed
+ *     2    Special case of 9223372036854775808
+ */
+int
+sql_atoi64(const char *z, int64_t *val, int length);
+
+/**
+ * Transform a UTF-8 integer literal, in either decimal or hexadecimal,
+ * into a 64-bit signed integer.  This routine accepts hexadecimal literals,
+ * whereas sql_atoi64() does not.
+ *
+ * @param z Literal being parsed.
+ * @param[out] val Parsed value.
+ * @retval
+ *     0    Successful transformation.  Fits in a 64-bit signed integer.
+ *     1    Integer too large for a 64-bit signed integer or is malformed
+ *     2    Special case of 9223372036854775808
+ */
+int
+sql_dec_or_hex_to_i64(const char *z, int64_t *val);
+
 void sqlite3ErrorWithMsg(sqlite3 *, int, const char *, ...);
 void sqlite3Error(sqlite3 *, int);
 void sqlite3SystemError(sqlite3 *, int);
diff --git a/src/box/sql/util.c b/src/box/sql/util.c
index e4c2c5d..d6e750c 100644
--- a/src/box/sql/util.c
+++ b/src/box/sql/util.c
@@ -621,27 +621,35 @@ compare2pow63(const char *zNum, int incr)
        return c;
 }
 
-/*
- * Convert zNum to a 64-bit signed integer.  zNum must be decimal. This
+/**
+ * Convert z to a 64-bit signed integer.  z must be decimal. This
  * routine does *not* accept hexadecimal notation.
  *
- * If the zNum value is representable as a 64-bit twos-complement
- * integer, then write that value into *pNum and return 0.
+ * If the z value is representable as a 64-bit twos-complement
+ * integer, then write that value into *val and return 0.
  *
- * If zNum is exactly 9223372036854775808, return 2.  This special
+ * If z is exactly 9223372036854775808, return 2.  This special
  * case is broken out because while 9223372036854775808 cannot be a
  * signed 64-bit integer, its negative -9223372036854775808 can be.
  *
- * If zNum is too big for a 64-bit integer and is not
- * 9223372036854775808  or if zNum contains any non-numeric text,
+ * If z is too big for a 64-bit integer and is not
+ * 9223372036854775808  or if z contains any non-numeric text,
  * then return 1.
  *
  * length is the number of bytes in the string (bytes, not characters).
  * The string is not necessarily zero-terminated.  The encoding is
  * given by enc.
+ *
+ * @param z String being parsed.
+ * @param[out] val Output integer value.
+ * @param length String length in bytes.
+ * @retval
+ *     0    Successful transformation.  Fits in a 64-bit signed integer.
+ *     1    Integer too large for a 64-bit signed integer or is malformed
+ *     2    Special case of 9223372036854775808
  */
 int
-sqlite3Atoi64(const char *zNum, i64 * pNum, int length)
+sql_atoi64(const char *z, int64_t *val, int length)
 {
        int incr = 1; // UTF-8
        u64 u = 0;
@@ -650,38 +658,35 @@ sqlite3Atoi64(const char *zNum, i64 * pNum, int length)
        int c = 0;
        int nonNum = 0;         /* True if input contains UTF16 with high byte 
non-zero */
        const char *zStart;
-       const char *zEnd = zNum + length;
+       const char *zEnd = z + length;
        incr = 1;
-       while (zNum < zEnd && sqlite3Isspace(*zNum))
-               zNum += incr;
-       if (zNum < zEnd) {
-               if (*zNum == '-') {
+       while (z < zEnd && sqlite3Isspace(*z))
+               z += incr;
+       if (z < zEnd) {
+               if (*z == '-') {
                        neg = 1;
-                       zNum += incr;
-               } else if (*zNum == '+') {
-                       zNum += incr;
+                       z += incr;
+               } else if (*z == '+') {
+                       z += incr;
                }
        }
-       zStart = zNum;
-       while (zNum < zEnd && zNum[0] == '0') {
-               zNum += incr;
+       zStart = z;
+       while (z < zEnd && z[0] == '0') {
+               z += incr;
        }                       /* Skip leading zeros. */
-       for (i = 0; &zNum[i] < zEnd && (c = zNum[i]) >= '0' && c <= '9';
+       for (i = 0; &z[i] < zEnd && (c = z[i]) >= '0' && c <= '9';
             i += incr) {
                u = u * 10 + c - '0';
        }
        if (u > LARGEST_INT64) {
-               *pNum = neg ? SMALLEST_INT64 : LARGEST_INT64;
+               *val = neg ? SMALLEST_INT64 : LARGEST_INT64;
        } else if (neg) {
-               *pNum = -(i64) u;
+               *val = -(i64) u;
        } else {
-               *pNum = (i64) u;
+               *val = (i64) u;
        }
-       testcase(i == 18);
-       testcase(i == 19);
-       testcase(i == 20);
-       if (&zNum[i] < zEnd     /* Extra bytes at the end */
-           || (i == 0 && zStart == zNum)       /* No digits */
+       if (&z[i] < zEnd        /* Extra bytes at the end */
+           || (i == 0 && zStart == z)  /* No digits */
            ||i > 19 * incr     /* Too many digits */
            || nonNum           /* UTF16 with high-order bytes non-zero */
            ) {
@@ -695,7 +700,7 @@ sqlite3Atoi64(const char *zNum, i64 * pNum, int length)
                return 0;
        } else {
                /* zNum is a 19-digit numbers.  Compare it against 
9223372036854775808. */
-               c = compare2pow63(zNum, incr);
+               c = compare2pow63(z, incr);
                if (c < 0) {
                        /* zNum is less than 9223372036854775808 so it fits */
                        assert(u <= LARGEST_INT64);
@@ -713,36 +718,19 @@ sqlite3Atoi64(const char *zNum, i64 * pNum, int length)
        }
 }
 
-/*
- * Transform a UTF-8 integer literal, in either decimal or hexadecimal,
- * into a 64-bit signed integer.  This routine accepts hexadecimal literals,
- * whereas sqlite3Atoi64() does not.
- *
- * Returns:
- *
- *     0    Successful transformation.  Fits in a 64-bit signed integer.
- *     1    Integer too large for a 64-bit signed integer or is malformed
- *     2    Special case of 9223372036854775808
- */
 int
-sqlite3DecOrHexToI64(const char *z, i64 * pOut)
+sql_dec_or_hex_to_i64(const char *z, int64_t *val)
 {
-#ifndef SQLITE_OMIT_HEX_INTEGER
-       if (z[0] == '0' && (z[1] == 'x' || z[1] == 'X')
-           ) {
-               u64 u = 0;
+       if (z[0] == '0' && (z[1] == 'x' || z[1] == 'X')) {
+               uint64_t u = 0;
                int i, k;
-               for (i = 2; z[i] == '0'; i++) {
-               }
-               for (k = i; sqlite3Isxdigit(z[k]); k++) {
+               for (i = 2; z[i] == '0'; i++);
+               for (k = i; sqlite3Isxdigit(z[k]); k++)
                        u = u * 16 + sqlite3HexToInt(z[k]);
-               }
-               memcpy(pOut, &u, 8);
+               memcpy(val, &u, 8);
                return (z[k] == 0 && k - i <= 16) ? 0 : 1;
-       } else
-#endif                         /* SQLITE_OMIT_HEX_INTEGER */
-       {
-               return sqlite3Atoi64(z, pOut, sqlite3Strlen30(z));
+       } else {
+               return sql_atoi64(z, val, sqlite3Strlen30(z));
        }
 }
 
@@ -768,7 +756,6 @@ sqlite3GetInt32(const char *zNum, int *pValue)
        } else if (zNum[0] == '+') {
                zNum++;
        }
-#ifndef SQLITE_OMIT_HEX_INTEGER
        else if (zNum[0] == '0' && (zNum[1] == 'x' || zNum[1] == 'X')
                 && sqlite3Isxdigit(zNum[2])
            ) {
@@ -786,7 +773,6 @@ sqlite3GetInt32(const char *zNum, int *pValue)
                        return 0;
                }
        }
-#endif
        while (zNum[0] == '0')
                zNum++;
        for (i = 0; i < 11 && (c = zNum[i] - '0') >= 0 && c <= 9; i++) {
diff --git a/src/box/sql/vdbe.c b/src/box/sql/vdbe.c
index f50e389..195638e 100644
--- a/src/box/sql/vdbe.c
+++ b/src/box/sql/vdbe.c
@@ -291,7 +291,7 @@ applyNumericAffinity(Mem *pRec, int bTryForInt)
        i64 iValue;
        assert((pRec->flags & (MEM_Str|MEM_Int|MEM_Real))==MEM_Str);
        if (sqlite3AtoF(pRec->z, &rValue, pRec->n)==0) return;
-       if (0==sqlite3Atoi64(pRec->z, &iValue, pRec->n)) {
+       if (0 == sql_atoi64(pRec->z, (int64_t *)&iValue, pRec->n)) {
                pRec->u.i = iValue;
                pRec->flags |= MEM_Int;
        } else {
@@ -389,12 +389,10 @@ static u16 SQLITE_NOINLINE computeNumericType(Mem *pMem)
 {
        assert((pMem->flags & (MEM_Int|MEM_Real))==0);
        assert((pMem->flags & (MEM_Str|MEM_Blob))!=0);
-       if (sqlite3AtoF(pMem->z, &pMem->u.r, pMem->n)==0) {
+       if (sqlite3AtoF(pMem->z, &pMem->u.r, pMem->n)==0)
                return 0;
-       }
-       if (sqlite3Atoi64(pMem->z, &pMem->u.i, pMem->n)==SQLITE_OK) {
+       if (sql_atoi64(pMem->z, (int64_t *)&pMem->u.i, pMem->n)==SQLITE_OK)
                return MEM_Int;
-       }
        return MEM_Real;
 }
 
diff --git a/src/box/sql/vdbemem.c b/src/box/sql/vdbemem.c
index 2ce9074..5d92a27 100644
--- a/src/box/sql/vdbemem.c
+++ b/src/box/sql/vdbemem.c
@@ -459,9 +459,9 @@ sqlite3VdbeIntValue(Mem * pMem)
        } else if (flags & MEM_Real) {
                return doubleToInt64(pMem->u.r);
        } else if (flags & (MEM_Str | MEM_Blob)) {
-               i64 value = 0;
+               int64_t value = 0;
                assert(pMem->z || pMem->n == 0);
-               sqlite3Atoi64(pMem->z, &value, pMem->n);
+               sql_atoi64(pMem->z, &value, pMem->n);
                return value;
        } else {
                return 0;
@@ -562,7 +562,7 @@ sqlite3VdbeMemNumerify(Mem * pMem)
 {
        if ((pMem->flags & (MEM_Int | MEM_Real | MEM_Null)) == 0) {
                assert((pMem->flags & (MEM_Blob | MEM_Str)) != 0);
-               if (0 == sqlite3Atoi64(pMem->z, &pMem->u.i, pMem->n)) {
+               if (0 == sql_atoi64(pMem->z, (int64_t *)&pMem->u.i, pMem->n)) {
                        MemSetTypeFlag(pMem, MEM_Int);
                } else {
                        pMem->u.r = sqlite3VdbeRealValue(pMem);
diff --git a/test/sql-tap/default.test.lua b/test/sql-tap/default.test.lua
index 9d59767..2f08a51 100755
--- a/test/sql-tap/default.test.lua
+++ b/test/sql-tap/default.test.lua
@@ -146,14 +146,13 @@ test:do_execsql_test(
        d INT DEFAULT -2147483647,
        e INT DEFAULT -2147483648,
        f INT DEFAULT (-9223372036854775808),
-       g INT DEFAULT 9223372036854775808,
        h INT DEFAULT (-(-9223372036854775807))
        );
        INSERT INTO t300 DEFAULT VALUES;
-       SELECT a, b, c, d, e, f, cast(g as text), h FROM t300;
+       SELECT a, b, c, d, e, f, h FROM t300;
        ]], {
        -- <default-3.3>
-       2147483647, 2147483648, 9223372036854775807LL, -2147483647, 
-2147483648, -9223372036854775808LL, "9.22337203685478e+18", 
9223372036854775807LL
+       2147483647, 2147483648, 9223372036854775807LL, -2147483647, 
-2147483648, -9223372036854775808LL, 9223372036854775807LL
        -- </default-3.3>
 })
 
diff --git a/test/sql/gh-2347-max-int-literals.test.lua 
b/test/sql/gh-2347-max-int-literals.test.lua
new file mode 100644
index 0000000..4b1ef0d
--- /dev/null
+++ b/test/sql/gh-2347-max-int-literals.test.lua
@@ -0,0 +1,11 @@
+test_run = require('test_run').new()
+engine = test_run:get_cfg('engine')
+box.sql.execute('pragma sql_default_engine=\''..engine..'\'')
+
+box.cfg{}
+
+box.sql.execute("select (9223372036854775807)")
+box.sql.execute("select (-9223372036854775808)")
+
+box.sql.execute("select (9223372036854775808)")
+box.sql.execute("select (-9223372036854775809)")
-- 
2.16.2


Other related posts: