sqlite: backport targeted security fixes for issue #3904

sqlite/SECURITY_PATCHSET_3904.md

@@ -0,0 +1,28 @@
+# Issue #3904 SQLite Security Backport Matrix
+
+Scope: targeted `src/*` backports from upstream SQLite commits referenced in
+https://github.com/bloomberg/comdb2/issues/3904.
+
+## Status Summary
+
+| Order | Upstream Commit | URL | Local Targets | Status | Verification Notes |
+|---|---|---|---|---|---|
+| 1 | `1e9c47be1e81e94a67f788c98fd70e8bf70e3746` | https://github.com/sqlite/sqlite/commit/1e9c47be1e81e94a67f788c98fd70e8bf70e3746 | `sqlite/src/build.c`, `sqlite/src/parse.y`, `sqlite/src/prepare.c` | `already_present` | Behavior is already in tree: `sqlite/src/build.c:2500`, `sqlite/src/parse.y:276`, `sqlite/src/prepare.c:52`. |
+| 2 | `522ebfa7cee96fb325a22ea3a2464a63485886a8` | https://github.com/sqlite/sqlite/commit/522ebfa7cee96fb325a22ea3a2464a63485886a8 | `sqlite/src/resolve.c` | `not_applicable` | This tree does not define generated-column symbols (`TF_HasGenerated`, `COLFLAG_GENERATED`); generated-column SQL is rejected (`tests/sqlite_bugs.test/t01.expected`). Guarded references are at `sqlite/src/resolve.c:593` and `sqlite/src/resolve.c:644`. |
+| 3 | `e59c562b3f6894f84c715772c4b116d7b5c01348` | https://github.com/sqlite/sqlite/commit/e59c562b3f6894f84c715772c4b116d7b5c01348 | `sqlite/src/select.c` | `applied` | DISTINCT-to-GROUP rewrite now excludes window queries via `p->pWin==0` at `sqlite/src/select.c:6219`. |
+| 4 | `38096961c7cd109110ac21d3ed7dad7e0cb0ae06` | https://github.com/sqlite/sqlite/commit/38096961c7cd109110ac21d3ed7dad7e0cb0ae06 | `sqlite/src/alter.c`, `sqlite/src/build.c`, `sqlite/src/sqliteInt.h` | `applied_manual` | Added `SF_View` (`sqlite/src/sqliteInt.h:3052`), mark views in create path (`sqlite/src/build.c:2815`), and prune/unset in rename walkers (`sqlite/src/alter.c:817`, `sqlite/src/alter.c:1397`, `sqlite/src/alter.c:1482`). |
+| 5 | `ebd70eedd5d6e6a890a670b5ee874a5eae86b4dd` | https://github.com/sqlite/sqlite/commit/ebd70eedd5d6e6a890a670b5ee874a5eae86b4dd | `sqlite/src/pragma.c` | `applied` | Guard added so `OPFLAG_TYPEOFARG` is only set if last opcode is `OP_Column`: `sqlite/src/pragma.c:1580`. |
+| 6 | `926f796e8feec15f3836aa0a060ed906f8ae04d3` | https://github.com/sqlite/sqlite/commit/926f796e8feec15f3836aa0a060ed906f8ae04d3 | `sqlite/src/resolve.c` | `not_applicable` | Same rationale as commit 2: generated-column support is absent in this baseline, so upstream generated-column behavior is not reachable. Fallback `colUsed` logic remains active at `sqlite/src/resolve.c:659`. |
+| 7 | `396afe6f6aa90a31303c183e11b2b2d4b7956b35` | https://github.com/sqlite/sqlite/commit/396afe6f6aa90a31303c183e11b2b2d4b7956b35 | `sqlite/src/select.c` | `applied_manual` | LEFT JOIN flattening guard now blocks DISTINCT parents: `sqlite/src/select.c:3855`. |
+| 8 | `8428b3b437569338a9d1e10c4cd8154acbe33089` | https://github.com/sqlite/sqlite/commit/8428b3b437569338a9d1e10c4cd8154acbe33089 | `sqlite/src/select.c` | `applied` | Early abort on prior parse errors in `multiSelect()` at `sqlite/src/select.c:2878`. |
+| 9 | `a6c1a71cde082e09750465d5675699062922e387` | https://github.com/sqlite/sqlite/commit/a6c1a71cde082e09750465d5675699062922e387 | `sqlite/src/select.c` | `applied` | `selectExpander()` now checks `pParse->nErr` before join processing at `sqlite/src/select.c:5055`. |
+| 10 | `0934d640456bb168a8888ae388643c5160afe501` | https://github.com/sqlite/sqlite/commit/0934d640456bb168a8888ae388643c5160afe501 | `sqlite/src/expr.c` | `applied` | Defensive aggregate bounds checks in expression code at `sqlite/src/expr.c:3882`. |
+
+## Notes
+
+- Patches were applied to `sqlite/src/*` only. Upstream `manifest*` metadata
+  changes were intentionally excluded.
+- Two commits required semantic/manual adaptation due local divergence:
+  `380969...` and `396afe...`.
+- Generated-column specific fixes (`522eb...`, `926f...`) are not applicable on
+  this baseline because generated-column support is not compiled in.

sqlite/src/alter.c

@@ -814,6 +814,7 @@ static void renameWalkWith(Walker *pWalker, Select *pSelect){
 ** descend into sub-select statements.
 */
 static int renameColumnSelectCb(Walker *pWalker, Select *p){
+  if( p->selFlags & SF_View ) return WRC_Prune;
   renameWalkWith(pWalker, p);
   return WRC_Continue;
 }
@@ -1279,8 +1280,9 @@ static void renameColumnFunc(
   if( sParse.pNewTable ){
     Select *pSelect = sParse.pNewTable->pSelect;
     if( pSelect ){
+      pSelect->selFlags &= ~SF_View;
       sParse.rc = SQLITE_OK;
-      sqlite3SelectPrep(&sParse, sParse.pNewTable->pSelect, 0);
+      sqlite3SelectPrep(&sParse, pSelect, 0);
       rc = (db->mallocFailed ? SQLITE_NOMEM : sParse.rc);
       if( rc==SQLITE_OK ){
         sqlite3WalkSelect(&sWalker, pSelect);
@@ -1392,6 +1394,7 @@ static int renameTableSelectCb(Walker *pWalker, Select *pSelect){
   int i;
   RenameCtx *p = pWalker->u.pRename;
   SrcList *pSrc = pSelect->pSrc;
+  if( pSelect->selFlags & SF_View ) return WRC_Prune;
   if( pSrc==0 ){
     assert( pWalker->pParse->db->mallocFailed );
     return WRC_Abort;
@@ -1471,13 +1474,16 @@ static void renameTableFunc(
 
         if( pTab->pSelect ){
           if( isLegacy==0 ){
+            Select *pSelect = pTab->pSelect;
             NameContext sNC;
             memset(&sNC, 0, sizeof(sNC));
             sNC.pParse = &sParse;
 
-            sqlite3SelectPrep(&sParse, pTab->pSelect, &sNC);
+            assert( pSelect->selFlags & SF_View );
+            pSelect->selFlags &= ~SF_View;
+            sqlite3SelectPrep(&sParse, pSelect, &sNC);
             if( sParse.nErr ) rc = sParse.rc;
-            sqlite3WalkSelect(&sWalker, pTab->pSelect);
+            sqlite3WalkSelect(&sWalker, pSelect);
           }
         }else{
           /* Modify any FK definitions to point to the new table. */

sqlite/src/build.c

@@ -2812,6 +2812,7 @@ void sqlite3CreateView(
   ** allocated rather than point to the input string - which means that
   ** they will persist after the current sqlite3_exec() call returns.
   */
+  pSelect->selFlags |= SF_View;
   if( IN_RENAME_OBJECT ){
     p->pSelect = pSelect;
     pSelect = 0;

sqlite/src/expr.c

@@ -3614,7 +3614,10 @@ int sqlite3ExprCodeTarget(Parse *pParse, Expr *pExpr, int target){
   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;
@@ -3874,7 +3877,10 @@ int sqlite3ExprCodeTarget(Parse *pParse, Expr *pExpr, int target){
     }
     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{

sqlite/src/pragma.c

@@ -1576,7 +1576,9 @@ void sqlite3Pragma(
           if( j==pTab->iPKey ) continue;
           if( pTab->aCol[j].notNull==0 ) continue;
           sqlite3ExprCodeGetColumnOfTable(v, pTab, iDataCur, j, 3);
-          sqlite3VdbeChangeP5(v, OPFLAG_TYPEOFARG);
+          if( sqlite3VdbeGetOp(v,-1)->opcode==OP_Column ){
+            sqlite3VdbeChangeP5(v, OPFLAG_TYPEOFARG);
+          }
           jmp2 = sqlite3VdbeAddOp1(v, OP_NotNull, 3); VdbeCoverage(v);
           zErr = sqlite3MPrintf(db, "NULL value in %s.%s", pTab->zName,
                               pTab->aCol[j].zName);

sqlite/src/resolve.c

@@ -569,17 +569,33 @@ static int lookupName(
 
   /* If a column from a table in pSrcList is referenced, then record
   ** this fact in the pSrcList.a[].colUsed bitmask.  Column 0 causes
-  ** bit 0 to be set.  Column 1 sets bit 1.  And so forth.  If the
-  ** column number is greater than the number of bits in the bitmask
-  ** then set the high-order bit of the bitmask.
+  ** bit 0 to be set.  Column 1 sets bit 1.  And so forth.
+  **
+  ** The colUsed mask is an optimization used to help determine if an
+  ** index is a covering index.  The correct answer is still obtained
+  ** if the mask contains extra bits.  But omitting bits from the mask
+  ** might result in an incorrect answer.
+  **
+  ** The high-order bit of the mask is a "we-use-them-all" bit.
+  ** If the column number is greater than the number of bits in the bitmask
+  ** then set the high-order bit of the bitmask.  If generated columns are
+  ** supported, also set the high-order bit for generated-column references,
+  ** as that adds dependencies that are difficult to track.
   */
   if( pExpr->iColumn>=0 && pMatch!=0 ){
     int n = pExpr->iColumn;
     testcase( n==BMS-1 );
     if( n>=BMS ){
       n = BMS-1;
     }
+    assert( pExpr->y.pTab!=0 );
     assert( pMatch->iCursor==pExpr->iTable );
+#if defined(TF_HasGenerated) && defined(COLFLAG_GENERATED)
+    if( pExpr->y.pTab->tabFlags & TF_HasGenerated ){
+      Column *pCol = pExpr->y.pTab->aCol + pExpr->iColumn;
+      if( pCol->colFlags & COLFLAG_GENERATED ) n = BMS-1;
+    }
+#endif
     pMatch->colUsed |= ((Bitmask)1)<<n;
   }
 
@@ -625,9 +641,26 @@ Expr *sqlite3CreateColumnExpr(sqlite3 *db, SrcList *pSrc, int iSrc, int iCol){
       p->iColumn = -1;
     }else{
       p->iColumn = (ynVar)iCol;
-      testcase( iCol==BMS );
-      testcase( iCol==BMS-1 );
-      pItem->colUsed |= ((Bitmask)1)<<(iCol>=BMS ? BMS-1 : iCol);
+      #if defined(TF_HasGenerated) && defined(COLFLAG_GENERATED)
+      Table *pTab = p->y.pTab;
+      if( pTab->tabFlags & TF_HasGenerated ){
+        Column *pColumn = pTab->aCol + iCol;
+        if( pColumn->colFlags & COLFLAG_GENERATED ){
+          testcase( pTab->nCol==63 );
+          testcase( pTab->nCol==64 );
+          if( pTab->nCol>=64 ){
+            pItem->colUsed = ALLBITS;
+          }else{
+            pItem->colUsed = MASKBIT(pTab->nCol)-1;
+          }
+        }
+      }else
+      #endif
+      {
+        testcase( iCol==BMS );
+        testcase( iCol==BMS-1 );
+        pItem->colUsed |= ((Bitmask)1)<<(iCol>=BMS ? BMS-1 : iCol);
+      }
     }
   }
   return p;

sqlite/src/select.c

@@ -2875,6 +2875,7 @@ static int multiSelect(
     }
   #endif
   }
+  if( pParse->nErr ) goto multi_select_end;
 
   /* Compute collating sequences used by 
   ** temporary tables needed to implement the compound select.
@@ -3830,13 +3831,16 @@ static int flattenSubquery(
   **
   ** which is not at all the same thing.
   **
-  ** If the subquery is the right operand of a LEFT JOIN, then the outer
-  ** query cannot be an aggregate. (3c)  This is an artifact of the way
-  ** aggregates are processed - there is no mechanism to determine if
-  ** the LEFT JOIN table should be all-NULL.
-  **
-  ** See also tickets #306, #350, and #3300.
-  */
+** If the subquery is the right operand of a LEFT JOIN, then the outer
+** query cannot be an aggregate. (3c)  This is an artifact of the way
+** aggregates are processed - there is no mechanism to determine if
+** the LEFT JOIN table should be all-NULL.
+**
+** If the subquery is the right operand of a LEFT JOIN, then the outer
+** query may not be DISTINCT.  See ticket [862974312edf00e9].
+**
+** See also tickets #306, #350, and #3300.
+*/
   if( (pSubitem->fg.jointype & JT_OUTER)!=0 ){
 #if defined(SQLITE_BUILDING_FOR_COMDB2)
     extern int gbl_enable_sq_flattening_optimization;
@@ -3845,8 +3849,11 @@ static int flattenSubquery(
     }
 #endif /* defined(SQLITE_BUILDING_FOR_COMDB2) */
     isLeftJoin = 1;
-    if( pSubSrc->nSrc>1 || isAgg || IsVirtual(pSubSrc->a[0].pTab) ){
-      /*  (3a)             (3c)     (3b) */
+    if( pSubSrc->nSrc>1                   /* (3a) */
+     || isAgg                             /* (3c) */
+     || IsVirtual(pSubSrc->a[0].pTab)     /* (3b) */
+     || (p->selFlags & SF_Distinct)!=0    /* (3d) */
+    ){
       return 0;
     }
   }
@@ -5045,7 +5052,7 @@ static int selectExpander(Walker *pWalker, Select *p){
 
   /* Process NATURAL keywords, and ON and USING clauses of joins.
   */
-  if( db->mallocFailed || sqliteProcessJoin(pParse, p) ){
+  if( pParse->nErr || db->mallocFailed || sqliteProcessJoin(pParse, p) ){
     return WRC_Abort;
   }
 
@@ -6209,6 +6216,7 @@ int sqlite3Select(
   */
   if( (p->selFlags & (SF_Distinct|SF_Aggregate))==SF_Distinct 
    && sqlite3ExprListCompare(sSort.pOrderBy, pEList, -1)==0
+   && p->pWin==0
   ){
     p->selFlags &= ~SF_Distinct;
     pGroupBy = p->pGroupBy = sqlite3ExprListDup(db, pEList, 0);

sqlite/src/sqliteInt.h

@@ -3049,6 +3049,7 @@ struct Select {
 #define SF_Converted      0x10000  /* By convertCompoundSelectToSubquery() */
 #define SF_IncludeHidden  0x20000  /* Include hidden columns in output */
 #define SF_ComplexResult  0x40000  /* Result contains subquery or function */
+#define SF_View          0x0800000 /* SELECT statement is a view */
 #define SF_NoopOrderBy   0x0400000 /* ORDER BY is ignored for this query */
 #if defined(SQLITE_BUILDING_FOR_COMDB2)
 #define SF_ASTIncluded  0x1000000  /* AST Generated */

tests/sqlite_bugs.test/t01.expected

@@ -0,0 +1,17 @@
+(test='---- test#2 ----')
+[CREATE TABLE gc0(c0, c1 NOT NULL AS (c0==0))] failed with rc -3 No type specified.
+[INSERT INTO gc0(c0) VALUES(0)] failed with rc -3 no such table: gc0
+[PRAGMA integrity_check] failed with rc -3 not authorized
+[CREATE TABLE gc1(x, y AS(x+1))] failed with rc -3 No type specified.
+[INSERT INTO gc1 VALUES(10)] failed with rc -3 no such table: gc1
+[SELECT y FROM gc1 JOIN gc1 USING (y,y)] failed with rc -3 no such table: gc1
+[CREATE INDEX gc1y ON gc1(y)] failed with rc -3 Table not found
+[SELECT y FROM gc1 JOIN gc1 USING (y,y)] failed with rc -3 no such table: gc1
+[CREATE TABLE gc3(aa INT PRIMARY KEY, bb UNIQUE AS(aa))] failed with rc -3 No type specified.
+[INSERT INTO gc3 VALUES(1)] failed with rc -3 no such table: gc3
+[SELECT 100 AS tag, aa, bb FROM gc3] failed with rc -3 no such table: gc3
+[DELETE FROM gc3 WHERE (SELECT bb FROM gc3)] failed with rc -3 no such table: gc3
+[SELECT 200 AS tag, aa, bb FROM gc3] failed with rc -3 no such table: gc3
+[DROP TABLE gc0] failed with rc -3 no such table: gc0
+[DROP TABLE gc1] failed with rc -3 no such table: gc1
+[DROP TABLE gc3] failed with rc -3 no such table: gc3

tests/sqlite_bugs.test/t01.sql

@@ -0,0 +1,25 @@
+SELECT '---- test#2 ----' as test;
+
+# Generated-column integrity_check and colUsed regressions:
+# - https://github.com/sqlite/sqlite/commit/ebd70eedd5d6e6a890a670b5ee874a5eae86b4dd
+# - https://github.com/sqlite/sqlite/commit/522ebfa7cee96fb325a22ea3a2464a63485886a8
+# - https://github.com/sqlite/sqlite/commit/926f796e8feec15f3836aa0a060ed906f8ae04d3
+CREATE TABLE gc0(c0, c1 NOT NULL AS (c0==0))$$
+INSERT INTO gc0(c0) VALUES(0);
+PRAGMA integrity_check;
+
+CREATE TABLE gc1(x, y AS(x+1))$$
+INSERT INTO gc1 VALUES(10);
+SELECT y FROM gc1 JOIN gc1 USING (y,y);
+CREATE INDEX gc1y ON gc1(y);
+SELECT y FROM gc1 JOIN gc1 USING (y,y);
+
+CREATE TABLE gc3(aa INT PRIMARY KEY, bb UNIQUE AS(aa))$$
+INSERT INTO gc3 VALUES(1);
+SELECT 100 AS tag, aa, bb FROM gc3;
+DELETE FROM gc3 WHERE (SELECT bb FROM gc3);
+SELECT 200 AS tag, aa, bb FROM gc3;
+
+DROP TABLE gc0;
+DROP TABLE gc1;
+DROP TABLE gc3;

tests/sqlite_bugs.test/t02.expected

@@ -0,0 +1,2 @@
+(test='---- test#3 ----')
+(name='t4')

tests/sqlite_bugs.test/t02.sql

@@ -0,0 +1,12 @@
+SELECT '---- test#3 ----' as test;
+
+# View/CTE ALTER handling:
+# - https://github.com/sqlite/sqlite/commit/38096961c7cd109110ac21d3ed7dad7e0cb0ae06
+# - https://github.com/sqlite/sqlite/commit/a6c1a71cde082e09750465d5675699062922e387
+CREATE TABLE t1(a int)$$
+CREATE VIEW v2(b) AS WITH t3(b) AS (SELECT 1) SELECT b FROM t3$$
+ALTER TABLE t1 RENAME TO t4$$
+SELECT name FROM sqlite_master WHERE type='table' AND name IN ('t1','t4') ORDER BY name;
+
+DROP VIEW v2;
+DROP TABLE t4;

tests/sqlite_bugs.test/t03.expected

@@ -0,0 +1,5 @@
+(test='---- test#4 ----')
+(rows inserted=1)
+(rows inserted=1)
+(rows inserted=1)
+(s=6, x=1)

tests/sqlite_bugs.test/t03.sql

@@ -0,0 +1,15 @@
+SELECT '---- test#4 ----' as test;
+
+# DISTINCT + window-function crash path:
+# - https://github.com/sqlite/sqlite/commit/e59c562b3f6894f84c715772c4b116d7b5c01348
+# - https://github.com/sqlite/sqlite/commit/8428b3b437569338a9d1e10c4cd8154acbe33089
+# - https://github.com/sqlite/sqlite/commit/0934d640456bb168a8888ae388643c5160afe501
+CREATE TABLE w1(aa int, bb int)$$
+INSERT INTO w1 VALUES(1,2);
+INSERT INTO w1 VALUES(5,6);
+CREATE TABLE w2(x int)$$
+INSERT INTO w2 VALUES(1);
+SELECT (SELECT DISTINCT sum(aa) OVER() FROM w1 ORDER BY 1) AS s, x FROM w2 ORDER BY 1;
+
+DROP TABLE w1;
+DROP TABLE w2;

tests/sqlite_bugs.test/t04.expected

@@ -0,0 +1,5 @@
+(test='---- test#5 ----')
+(rows inserted=1)
+(rows inserted=1)
+(rows inserted=1)
+(c=11)

tests/sqlite_bugs.test/t04.sql

@@ -0,0 +1,12 @@
+SELECT '---- test#5 ----' as test;
+
+# DISTINCT + LEFT JOIN flattening guard:
+# - https://github.com/sqlite/sqlite/commit/396afe6f6aa90a31303c183e11b2b2d4b7956b35
+CREATE TABLE j0(a int, b int)$$
+CREATE INDEX j0a ON j0(a);
+INSERT INTO j0 VALUES(10,10);
+INSERT INTO j0 VALUES(10,11);
+INSERT INTO j0 VALUES(10,12);
+SELECT DISTINCT c FROM j0 LEFT JOIN (SELECT a+1 AS c FROM j0) ORDER BY c;
+
+DROP TABLE j0;

tests/sqlite_bugs.test/t05.expected

@@ -0,0 +1,4 @@
+(test='---- test#6 ----')
+[CREATE TABLE ctas1 AS SELECT 1 AS x, 2 AS y] failed with rc -3 near "AS": syntax error
+[SELECT x, y FROM ctas1] failed with rc -3 no such table: ctas1
+[DROP TABLE ctas1] failed with rc -3 no such table: ctas1

tests/sqlite_bugs.test/t05.sql

@@ -0,0 +1,7 @@
+SELECT '---- test#6 ----' as test;
+
+# CREATE TABLE AS smoke path (related to schema handling around CTAS entries):
+# - https://github.com/sqlite/sqlite/commit/1e9c47be1e81e94a67f788c98fd70e8bf70e3746
+CREATE TABLE ctas1 AS SELECT 1 AS x, 2 AS y$$
+SELECT x, y FROM ctas1;
+DROP TABLE ctas1;