From: Nikita Pettik <korablev@tarantool.org> To: tarantool-patches@dev.tarantool.org Cc: v.shpilevoy@tarantool.org Subject: [Tarantool-patches] [PATCH v2 2/6] sql: fix possible null dereference in sql_expr_coll() Date: Wed, 11 Dec 2019 16:44:54 +0300 [thread overview] Message-ID: <ac49a6b2bda02339dbdd4e3ece87971f84847215.1576071711.git.korablev@tarantool.org> (raw) In-Reply-To: <cover.1576022424.git.korablev@tarantool.org> In-Reply-To: <cover.1576071711.git.korablev@tarantool.org> Some built-in functions can accept different number of arguments. Check of argument count for such functions takes place right before its execution. So it is possible that expression-list representing arguments for built-in function is NULL. On the other hand, in sql_expr_coll() (which returns collation of expression) it is assumed that if function features SQL_FUNC_DERIVEDCOLL flag (it implies that resulting collation depends on collation of arguments) then it has at least one argument. The last assumption is wrong considering for example SUBSTR() function: it may have 1 or 2 arguments, so check of argument count doesn't occur during compilation. Hence, if it is required to calculate collation for SUBSTR() function and there's no arguments, Tarantool crashes due to null-dereference. This patch fixes this bug with one additional check in sql_expr_coll(). --- src/box/sql/expr.c | 3 ++- test/sql/collation.result | 9 +++++++++ test/sql/collation.test.lua | 5 +++++ 3 files changed, 16 insertions(+), 1 deletion(-) diff --git a/src/box/sql/expr.c b/src/box/sql/expr.c index 648b7170e..0bdcfe576 100644 --- a/src/box/sql/expr.c +++ b/src/box/sql/expr.c @@ -332,7 +332,8 @@ sql_expr_coll(Parse *parse, Expr *p, bool *is_explicit_coll, uint32_t *coll_id, sql_func_by_signature(p->u.zToken, arg_count); if (func == NULL) break; - if (sql_func_flag_is_set(func, SQL_FUNC_DERIVEDCOLL)) { + if (sql_func_flag_is_set(func, SQL_FUNC_DERIVEDCOLL) && + arg_count > 0) { /* * Now we use quite straightforward * approach assuming that resulting diff --git a/test/sql/collation.result b/test/sql/collation.result index 11962ef47..fbc7ce9aa 100644 --- a/test/sql/collation.result +++ b/test/sql/collation.result @@ -292,6 +292,15 @@ box.execute("SELECT * FROM t WHERE a COLLATE \"binary\" = c COLLATE \"unicode\"; - null - Illegal mix of collations ... +-- Make sure that using function featuring variable arguemnts +-- length and resulting collation which depends on arguments +-- is processed correctly. +-- +box.execute("SELECT * FROM t WHERE a COLLATE \"binary\" = substr();") +--- +- null +- 'Wrong number of arguments is passed to SUBSTR(): expected 1 or 2, got 0' +... -- Compound queries perform implicit comparisons between values. -- Hence, rules for collations compatibilities are the same. -- diff --git a/test/sql/collation.test.lua b/test/sql/collation.test.lua index 1be28b3ff..a013253cd 100644 --- a/test/sql/collation.test.lua +++ b/test/sql/collation.test.lua @@ -80,6 +80,11 @@ box.execute("SELECT * FROM t WHERE b = c;") box.execute("SELECT * FROM t WHERE b COLLATE \"binary\" = c;") box.execute("SELECT * FROM t WHERE a = c;") box.execute("SELECT * FROM t WHERE a COLLATE \"binary\" = c COLLATE \"unicode\";") +-- Make sure that using function featuring variable arguemnts +-- length and resulting collation which depends on arguments +-- is processed correctly. +-- +box.execute("SELECT * FROM t WHERE a COLLATE \"binary\" = substr();") -- Compound queries perform implicit comparisons between values. -- Hence, rules for collations compatibilities are the same. -- 2.15.1
next prev parent reply other threads:[~2019-12-11 13:45 UTC|newest] Thread overview: 25+ messages / expand[flat|nested] mbox.gz Atom feed top 2019-12-11 13:44 [Tarantool-patches] [PATCH v2 0/6] sql: extended metadata Nikita Pettik [not found] ` <cover.1576071711.git.korablev@tarantool.org> 2019-12-11 13:44 ` [Tarantool-patches] [PATCH v2 1/6] sql: refactor resulting set metadata Nikita Pettik 2019-12-11 13:44 ` Nikita Pettik [this message] 2019-12-11 13:44 ` [Tarantool-patches] [PATCH v2 3/6] sql: extend result set with collation Nikita Pettik 2019-12-18 0:29 ` Vladislav Shpilevoy 2019-12-24 0:26 ` Nikita Pettik 2019-12-24 15:30 ` Vladislav Shpilevoy 2019-12-11 13:44 ` [Tarantool-patches] [PATCH v2 4/6] sql: extend result set with nullability Nikita Pettik 2019-12-18 0:29 ` Vladislav Shpilevoy 2019-12-24 0:26 ` Nikita Pettik 2019-12-11 13:44 ` [Tarantool-patches] [PATCH v2 5/6] sql: extend result set with autoincrement Nikita Pettik 2019-12-18 0:29 ` Vladislav Shpilevoy 2019-12-24 0:26 ` Nikita Pettik 2019-12-24 15:30 ` Vladislav Shpilevoy 2019-12-25 12:17 ` Nikita Pettik 2019-12-25 15:40 ` Vladislav Shpilevoy 2019-12-25 23:18 ` Nikita Pettik 2019-12-11 13:44 ` [Tarantool-patches] [PATCH v2 6/6] sql: extend result set with alias Nikita Pettik 2019-12-18 0:29 ` Vladislav Shpilevoy 2019-12-24 0:26 ` Nikita Pettik 2019-12-24 15:34 ` Vladislav Shpilevoy 2019-12-26 11:24 ` Nikita Pettik 2019-12-27 11:53 ` Vladislav Shpilevoy 2019-12-27 23:44 ` Nikita Pettik 2019-12-28 9:29 ` Vladislav Shpilevoy
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=ac49a6b2bda02339dbdd4e3ece87971f84847215.1576071711.git.korablev@tarantool.org \ --to=korablev@tarantool.org \ --cc=tarantool-patches@dev.tarantool.org \ --cc=v.shpilevoy@tarantool.org \ --subject='Re: [Tarantool-patches] [PATCH v2 2/6] sql: fix possible null dereference in sql_expr_coll()' \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: link
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox