SUBSTRING with an invalid start position causes a segmentation fault

[mrigger]

Consider the following statement:

SELECT SUBSTRING(0, 3, 0); -- segmentation fault

Unexpectedly, the statement causes a segmentation fault in release builds. In debug builds, ASan prints the following error:

sqlite> SELECT SUBSTRING(0, 3, 0);
=================================================================
==29926==ERROR: AddressSanitizer: negative-size-param: (size=-1)
    #0 0x7f32c3f3b878  (/usr/lib/x86_64-linux-gnu/libasan.so.5+0x9a878)
    #1 0x55da9b478d8e in substring_ascii_only duckdb/src/function/scalar/string/substring.cpp:15
    #2 0x55da9b4798d9 in substring_scalar_function duckdb/src/function/scalar/string/substring.cpp:42
    #3 0x55da9b479fff in operator() duckdb/src/function/scalar/string/substring.cpp:82
    #4 0x55da9b482896 in Execute<duckdb::string_t, int, int, duckdb::string_t, duckdb::substring_function(duckdb::DataChunk&, duckdb::ExpressionState&, duckdb::Vector&)::<lambda(duckdb::string_t, int, int)> > duckdb/src/include/duckdb/common/vector_operations/ternary_executor.hpp:60
    #5 0x55da9b47a35f in substring_function duckdb/src/function/scalar/string/substring.cpp:79
    #6 0x55da9b15cd01 in duckdb::ExpressionExecutor::Execute(duckdb::BoundFunctionExpression&, duckdb::ExpressionState*, duckdb::SelectionVector const*, unsigned long, duckdb::Vector&) duckdb/src/execution/expression_executor/execute_function.cpp:40
    #7 0x55da9aca7830 in duckdb::ExpressionExecutor::Execute(duckdb::Expression&, duckdb::ExpressionState*, duckdb::SelectionVector const*, unsigned long, duckdb::Vector&) duckdb/src/execution/expression_executor.cpp:146
    #8 0x55da9aca6ac9 in duckdb::ExpressionExecutor::ExecuteExpression(unsigned long, duckdb::Vector&) duckdb/src/execution/expression_executor.cpp:71
    #9 0x55da9aca68dc in duckdb::ExpressionExecutor::ExecuteExpression(duckdb::Vector&) duckdb/src/execution/expression_executor.cpp:65
    #10 0x55da9aca6c92 in duckdb::ExpressionExecutor::EvaluateScalar(duckdb::Expression&) duckdb/src/execution/expression_executor.cpp:80
    #11 0x55da9b343cfc in duckdb::ConstantFoldingRule::Apply(duckdb::LogicalOperator&, std::vector<duckdb::Expression*, std::allocator<duckdb::Expression*> >&, bool&) duckdb/src/optimizer/rule/constant_folding.cpp:38
    #12 0x55da9ae8c397 in duckdb::ExpressionRewriter::ApplyRules(duckdb::LogicalOperator&, std::vector<duckdb::Rule*, std::allocator<duckdb::Rule*> > const&, std::unique_ptr<duckdb::Expression, std::default_delete<duckdb::Expression> >, bool&) duckdb/src/optimizer/expression_rewriter.cpp:17
    #13 0x55da9ae8cf50 in duckdb::ExpressionRewriter::Apply(duckdb::LogicalOperator&) duckdb/src/optimizer/expression_rewriter.cpp:67
    #14 0x55da9ae8cb02 in duckdb::ExpressionRewriter::Apply(duckdb::LogicalOperator&) duckdb/src/optimizer/expression_rewriter.cpp:43
    #15 0x55da9ae8a41f in duckdb::Optimizer::Optimize(std::unique_ptr<duckdb::LogicalOperator, std::default_delete<duckdb::LogicalOperator> >) duckdb/src/optimizer/optimizer.cpp:45
    #16 0x55da9ad00834 in duckdb::ClientContext::CreatePreparedStatement(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, std::unique_ptr<duckdb::SQLStatement, std::default_delete<duckdb::SQLStatement> >) duckdb/src/main/client_context.cpp:181
    #17 0x55da9ad0406c in duckdb::ClientContext::RunStatementInternal(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, std::unique_ptr<duckdb::SQLStatement, std::default_delete<duckdb::SQLStatement> >, bool) duckdb/src/main/client_context.cpp:329
    #18 0x55da9ad04ad5 in duckdb::ClientContext::RunStatement(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, std::unique_ptr<duckdb::SQLStatement, std::default_delete<duckdb::SQLStatement> >, bool) duckdb/src/main/client_context.cpp:360
    #19 0x55da9ad02714 in duckdb::ClientContext::Prepare(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >) duckdb/src/main/client_context.cpp:274
    #20 0x55da9ad0e037 in duckdb::Connection::Prepare(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >) duckdb/src/main/connection.cpp:73
    #21 0x55da9a9d4bb6 in sqlite3_prepare_v2 duckdb/tools/sqlite3_api_wrapper/sqlite3_api_wrapper.cpp:140
    #22 0x55da9a9b01ff in shell_exec duckdb/tools/shell/shell.c:10099
    #23 0x55da9a9ca38d in runOneSqlLine duckdb/tools/shell/shell.c:15047
    #24 0x55da9a9cacc7 in process_input duckdb/tools/shell/shell.c:15149
    #25 0x55da9a9cd3d4 in main duckdb/tools/shell/shell.c:15817
    #26 0x7f32c3971b6a in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x26b6a)
    #27 0x55da9a98a7b9 in _start (duckdb/build/debug/duckdb_cli+0x8f07b9)

I found this based on commit ed92e9c.

[Mytherin]

Thanks :) Fixed in 184f515. This was a bug I introduced while fixing a previous bug.