Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

invalid memory address or nil pointer dereference in expression.inferCollation #53580

Closed
ycybfhb opened this issue May 27, 2024 · 3 comments · Fixed by #53716
Closed

invalid memory address or nil pointer dereference in expression.inferCollation #53580

ycybfhb opened this issue May 27, 2024 · 3 comments · Fixed by #53716

Comments

@ycybfhb
Copy link

ycybfhb commented May 27, 2024

Bug Report

Please answer these questions before submitting your issue. Thanks!

1. Minimal reproduce step (Required)

First execute the following valid.sql
valid.txt
Then a crash occurs when executing the error.sql below
error.txt

2. What did you expect to see? (Required)

Expect no crashes

3. What did you see instead (Required)

runtime error: invalid memory address or nil pointer dereference

tidb.log:

[2024/05/27 04:41:12.123 +00:00] [ERROR] [conn.go:1013] ["connection running loop panic"] [conn=1904214040] [session_alias=] [err="runtime error: invalid memory address or nil pointer dereference"] [stack="github.com/pingcap/tidb/pkg/server.(*clientConn).Run.func1
	/workspace/source/tidb/pkg/server/conn.go:1016
runtime.gopanic
	/usr/local/go/src/runtime/panic.go:914
github.com/pingcap/tidb/pkg/executor.(*Compiler).Compile.func1
	/workspace/source/tidb/pkg/executor/compiler.go:57
runtime.gopanic
	/usr/local/go/src/runtime/panic.go:914
runtime.panicmem
	/usr/local/go/src/runtime/panic.go:261
runtime.sigpanic
	/usr/local/go/src/runtime/signal_unix.go:861
github.com/pingcap/tidb/pkg/expression.inferCollation
	/workspace/source/tidb/pkg/expression/collation.go:411
github.com/pingcap/tidb/pkg/expression.CheckAndDeriveCollationFromExprs
	/workspace/source/tidb/pkg/expression/collation.go:305
github.com/pingcap/tidb/pkg/expression.ColumnSubstituteImpl
	/workspace/source/tidb/pkg/expression/util.go:493
github.com/pingcap/tidb/pkg/expression.ColumnSubstituteImpl
	/workspace/source/tidb/pkg/expression/util.go:450
github.com/pingcap/tidb/pkg/expression.ColumnSubstituteImpl
	/workspace/source/tidb/pkg/expression/util.go:483
github.com/pingcap/tidb/pkg/expression.ColumnSubstituteImpl
	/workspace/source/tidb/pkg/expression/util.go:450
github.com/pingcap/tidb/pkg/expression.ColumnSubstituteImpl
	/workspace/source/tidb/pkg/expression/util.go:483
github.com/pingcap/tidb/pkg/expression.ColumnSubstituteImpl
	/workspace/source/tidb/pkg/expression/util.go:483
github.com/pingcap/tidb/pkg/planner/core.BreakDownPredicates
	/workspace/source/tidb/pkg/planner/core/rule_predicate_push_down.go:394
github.com/pingcap/tidb/pkg/planner/core.(*LogicalProjection).ConvertOuterToInnerJoin
	/workspace/source/tidb/pkg/planner/core/rule_outer_to_inner_join.go:145
github.com/pingcap/tidb/pkg/planner/core.(*baseLogicalPlan).ConvertOuterToInnerJoin
	/workspace/source/tidb/pkg/planner/core/rule_outer_to_inner_join.go:68
github.com/pingcap/tidb/pkg/planner/core.(*baseLogicalPlan).ConvertOuterToInnerJoin
	/workspace/source/tidb/pkg/planner/core/rule_outer_to_inner_join.go:68
github.com/pingcap/tidb/pkg/planner/core.(*LogicalJoin).ConvertOuterToInnerJoin
	/workspace/source/tidb/pkg/planner/core/rule_outer_to_inner_join.go:105
github.com/pingcap/tidb/pkg/planner/core.(*LogicalSelection).ConvertOuterToInnerJoin
	/workspace/source/tidb/pkg/planner/core/rule_outer_to_inner_join.go:137
github.com/pingcap/tidb/pkg/planner/core.(*LogicalProjection).ConvertOuterToInnerJoin
	/workspace/source/tidb/pkg/planner/core/rule_outer_to_inner_join.go:147
github.com/pingcap/tidb/pkg/planner/core.(*baseLogicalPlan).ConvertOuterToInnerJoin
	/workspace/source/tidb/pkg/planner/core/rule_outer_to_inner_join.go:68
github.com/pingcap/tidb/pkg/planner/core.(*baseLogicalPlan).ConvertOuterToInnerJoin
	/workspace/source/tidb/pkg/planner/core/rule_outer_to_inner_join.go:68
github.com/pingcap/tidb/pkg/planner/core.(*baseLogicalPlan).ConvertOuterToInnerJoin
	/workspace/source/tidb/pkg/planner/core/rule_outer_to_inner_join.go:68
github.com/pingcap/tidb/pkg/planner/core.(*baseLogicalPlan).ConvertOuterToInnerJoin
	/workspace/source/tidb/pkg/planner/core/rule_outer_to_inner_join.go:68
github.com/pingcap/tidb/pkg/planner/core.(*convertOuterToInnerJoin).optimize
	/workspace/source/tidb/pkg/planner/core/rule_outer_to_inner_join.go:57
github.com/pingcap/tidb/pkg/planner/core.logicalOptimize
	/workspace/source/tidb/pkg/planner/core/optimizer.go:1005
github.com/pingcap/tidb/pkg/planner/core.doOptimize
	/workspace/source/tidb/pkg/planner/core/optimizer.go:289
github.com/pingcap/tidb/pkg/planner/core.DoOptimize
	/workspace/source/tidb/pkg/planner/core/optimizer.go:348
github.com/pingcap/tidb/pkg/planner.optimize
	/workspace/source/tidb/pkg/planner/optimize.go:503
github.com/pingcap/tidb/pkg/planner.Optimize
	/workspace/source/tidb/pkg/planner/optimize.go:334
github.com/pingcap/tidb/pkg/executor.(*Compiler).Compile
	/workspace/source/tidb/pkg/executor/compiler.go:99
github.com/pingcap/tidb/pkg/session.(*session).ExecuteStmt
	/workspace/source/tidb/pkg/session/session.go:2094
github.com/pingcap/tidb/pkg/server.(*TiDBContext).ExecuteStmt
	/workspace/source/tidb/pkg/server/driver_tidb.go:294
github.com/pingcap/tidb/pkg/server.(*clientConn).handleStmt
	/workspace/source/tidb/pkg/server/conn.go:2021
github.com/pingcap/tidb/pkg/server.(*clientConn).handleQuery
	/workspace/source/tidb/pkg/server/conn.go:1774
github.com/pingcap/tidb/pkg/server.(*clientConn).dispatch
	/workspace/source/tidb/pkg/server/conn.go:1348
github.com/pingcap/tidb/pkg/server.(*clientConn).Run
	/workspace/source/tidb/pkg/server/conn.go:1114
github.com/pingcap/tidb/pkg/server.(*Server).onConn
	/workspace/source/tidb/pkg/server/server.go:739"]

4. What is your TiDB version? (Required)

+------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------+
| tidb_version()                                                                                                                                                                                                                                                   |
+------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------+
| Release Version: v8.2.0-alpha-216-gfe5858b
Edition: Community
Git Commit Hash: fe5858b00cd63808ac414c6e102a353778b0aaa7
Git Branch: HEAD
UTC Build Time: 2024-05-23 01:44:42
GoVersion: go1.21.10
Race Enabled: false
Check Table Before Drop: false
Store: tikv |
+------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------+

We are the BASS team from the School of Cyber Science and Technology at Beihang University. Our main focus is on system software security, operating systems, and program analysis research, as well as the development of automated program testing frameworks for detecting software defects. Using our self-developed database vulnerability testing tool, we have identified the above-mentioned vulnerabilities in TiDB that may lead to database crashes.

@YangKeao
Copy link
Member

YangKeao commented May 31, 2024

A smaller reproduce:

create table t (col TEXT);
select
  1
from
  (
    select
      t.col as c0,
      46578369 as c1
    from
      t
  ) as t
where
  case
    when (
      t.c0 in (
        t.c0,
        cast(
          (cast(1 as unsigned) - cast(t.c1 as signed)) as char
        )
      )
    ) then 1
    else 2
  end;

It's because the NewFunctionInternal (which is about to create a new in function) in ColumnSubstituteImpl returns nil, and then the first argument of case changed to nil, and cause this panic. The backtrace of this example has a tiny difference with the one given in the issue. For the issue, it panics in the optimization to turn outer join into inner join. For this issue, it panics in the predicates push down optimization. They share the same logic, so I think it's fine to consider them as a single issue.

I propose to use NewFunction instead of NewFunctionInternal, and handle the returned error carefully. (Another option is to avoid returning error for builtinInStringSig, but I think it's not elegant). I'm not sure whether it's correct or not, and need further investigation.

@YangKeao
Copy link
Member

YangKeao commented May 31, 2024

Thanks again for the report 🍻, you have done a good job. I'm a little curious how does the "vulnerability testing tool" work? Is it a better/special fuzzing technology or does it have other strategy to find the bad query?

BTW, it'll be really really helpful / cool if it can simplify the found query. The simpler the query is, the easier it'll be for us to locate the issue 🤝 (I understand it's a difficult feature, as it usually requires deep knowledge to inspect the problem and give a simpler reproduction).

@ycybfhb
Copy link
Author

ycybfhb commented Jun 2, 2024

Hello, we are developing a new fuzz testing tool that incorporates a new strategy. We have also implemented a method to simplify the creation of table statements and query statements. When submitting bugs in the future, we will try to provide a simplified PoC.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants