Unfixed vulnerabilities along with the forked codebase of sqlite in src/box/sql/

Discussed in https://github.com/orgs/tarantool/discussions/8849

^{Originally posted by fullwaywang July 5, 2023}
Hi there. I am a researcher in security and vulnerabilities. Recently I have been studying CVEs in OSS and their existence in downstream projects.

While modeling CVEs in sqlite3 and searching for vulnerable, cognate code across GitHub, I found this project. The codes in src/box/sql/ turned out to be a copy of sqlite3 with the baseline version from which it forked unknown. However, the legal announcements within the code files had been replaced, and symbols like function names had been carefully modified. Many individual modifications had been made on this codebase, which made it hard to map the modified functions to the original.

After some manual examination, I found out some CVEs which had been fixed in upstream sqlite3 did exist in tarantool, judging from vulnerable code contexts. For example, a patch fixing CVE-2020-13435 is commit 0934d64 in function sqlite3ExprCodeTarget:

diff --git a/src/expr.c b/src/expr.c
index 83dd8b1ab..c5b678387 100644
--- a/src/expr.c
+++ b/src/expr.c
@@ -3811,7 +3811,10 @@ expr_code_doover:
   switch( op ){
     case TK_AGG_COLUMN: {
       AggInfo *pAggInfo = pExpr->pAggInfo;
-      struct AggInfo_col *pCol = &pAggInfo->aCol[pExpr->iAgg];
+      struct AggInfo_col *pCol;
+      assert( pAggInfo!=0 );
+      assert( pExpr->iAgg>=0 && pExpr->iAgg<pAggInfo->nColumn );
+      pCol = &pAggInfo->aCol[pExpr->iAgg];
       if( !pAggInfo->directMode ){
         assert( pCol->iMem>0 );
         return pCol->iMem;
@@ -4111,7 +4114,10 @@ expr_code_doover:
     }
     case TK_AGG_FUNCTION: {
       AggInfo *pInfo = pExpr->pAggInfo;
-      if( pInfo==0 ){
+      if( pInfo==0
+       || NEVER(pExpr->iAgg<0)
+       || NEVER(pExpr->iAgg>=pInfo->nFunc)
+      ){
         assert( !ExprHasProperty(pExpr, EP_IntValue) );
         sqlite3ErrorMsg(pParse, "misuse of aggregate: %s()", pExpr->u.zToken);
       }else{

Which is apparently a patch against out-of-bound accessing of array. Examining the context in function sqlExprCodeTarget I got:

    switch (op) {
    case TK_AGG_COLUMN:{
            AggInfo *pAggInfo = pExpr->pAggInfo;
            struct AggInfo_col *pCol = &pAggInfo->aCol[pExpr->iAgg];
            if (!pAggInfo->directMode) {
                assert(pCol->iMem > 0);
                return pCol->iMem;

Though the context differed, the defect made sense anyway.

Honestly speaking I am totally unfamiliar with this project, and the role of the codes in src/box/sql/ is not clear to me. Would you make some investigation into the existence and exploitability of sqlite3 vulnerabilities, and probably merge upstream patches or switch to the upstream codes in the future?

Start by examining the forked SQLite code under src/box/sql/, especially expr.c and sqlExprCodeTarget, and compare it with the upstream CVE-2020-13435 fix in commit 0934d64. Determine whether the reported vulnerabilities exist and are exploitable, then document whether upstream patches or replacing the forked code are appropriate.