A false negative (miss) in asyncpg-sqli ruleset

[kholia]

Code block:

def bad12(conn: Connection):
    sql_query = 'SELECT * FROM {}'.format(user_input)
    sql_query_copy = sql_query
    # ruleid: asyncpg-sqli
    cur = await conn.cursor(sql_query_copy)

Scan results:

$ semgrep --config asyncpg-sqli.yaml asyncpg-sqli.py
               
               
┌─────────────┐
│ Scan Status │
└─────────────┘
  Scanning 1 file tracked by git with 1 Code rule:
  Scanning 1 file.
  ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ 100% 0:00:00                                                                                                                        
                    
                    
┌──────────────────┐
│ 10 Code Findings │
└──────────────────┘
                                   
    asyncpg-sqli.py 
       asyncpg-sqli                                                                         
          Detected string concatenation with a non-literal variable in a asyncpg Python SQL statement.
          This could lead to SQL injection if the variable is user-controlled and not properly        
          sanitized. In order to prevent SQL injection, use parameterized queries or prepared         
          statements instead. You can create parameterized queries like so: 'conn.fetch("SELECT $1    
          FROM table", value)'. You can also create prepared statements with 'Connection.prepare':    
          'stmt = conn.prepare("SELECT $1 FROM table"); await stmt.fetch(user_value)'                 
                                                                                                      
           10┆ values = await conn.fetch(query)
            ⋮┆----------------------------------------
           17┆ cur = await conn.cursor(sql_query)
            ⋮┆----------------------------------------
           23┆ await connection.execute(sql_query)
            ⋮┆----------------------------------------
           30┆ await pool.fetch(sql_query)
            ⋮┆----------------------------------------
           37┆ await con.execute("SELECT name FROM users WHERE age=" + req.FormValue("age"))
            ⋮┆----------------------------------------
           44┆ await con.execute('SELECT * FROM {}'.format(user_input))
            ⋮┆----------------------------------------
           50┆ conn.execute('SELECT * FROM %s'%(user_input))
            ⋮┆----------------------------------------
           54┆ conn.fetchrow(f'SELECT * FROM {user_input}')
            ⋮┆----------------------------------------
           58┆ conn.execute(
           59┆ "insert into %s values (%%s, %%s)" % ext.quote_ident(table_name),[10, 20])
            ⋮┆----------------------------------------
           70┆ cur = conn.fetch(common.bad_query_1.format(user_input))

The SQLi probem in the sample code block is NOT detected. It seems the sql_query_copy = sql_query code statement throws off the existing asyncpg-sqli.yaml ruleset somehow.

Does Semgrep have support for static taint tracking analysis?

Thanks for the help.

[kholia]

I tried the following patch but it generates too many false positives:

diff --git a/python/lang/security/audit/sqli/asyncpg-sqli.yaml b/python/lang/security/audit/sqli/asyncpg-sqli.yaml
index 45f88f9e..7023b8de 100644
--- a/python/lang/security/audit/sqli/asyncpg-sqli.yaml
+++ b/python/lang/security/audit/sqli/asyncpg-sqli.yaml
@@ -38,6 +38,11 @@ rules:
         - pattern-inside: |
             $QUERY = $X + $Y
             ...
+        - pattern-inside: |
+            $QUERY = $X + $Y
+            ...
+            $QUERY_COPY = $QUERY
+            ...
         - pattern-inside: |
             $QUERY += $X
             ...
@@ -63,6 +68,7 @@ rules:
           $QUERY = '...' % ()
           ...
     - pattern: $CONN.$METHOD(..., $X + $Y, ...)
+    - pattern: $CONN.$METHOD(..., $QUERY_COPY, ...)
     - pattern: $CONN.$METHOD(..., $Y.format(...), ...)
     - pattern: $CONN.$METHOD(..., '...'.format(...), ...)
     - pattern: $CONN.$METHOD(..., '...' % (...), ...)

[inkz]

If I understand correctly the issue is resolved by: #3024, feel free to reopen if not

[kholia]

@inkz Hi - This issue is NOT solved by PR #3024.