SQL's Insert() and Update() Methods Used F-String Interpolation in u2share_batch_give_sugar

Affected Versions

The Vulnerability Explained

The SQL class in this codebase provided a thin wrapper around Python's sqlite3 module, but it undermined the library's built-in protections. Instead of using parameterized queries, the Insert() and Update() methods used Python f-strings to splice values directly into SQL command strings.

Here's the vulnerable Insert() method:

def Insert(self, column, value):
    try:
        sql = f'''INSERT INTO "main"."info" ({column}) VALUES ({value})'''
        logger.debug(sql)
        self.cursor.execute(sql)

And the vulnerable Update() method:

def Update(self, columnvalue, newcolumnvalue):
    try:
        sql = f'UPDATE "main"."info" SET {newcolumnvalue} WHERE {columnvalue}'
        logger.debug(sql)
        self.cursor.execute(sql)

The critical flaw: the execute() call receives a single string argument with values already interpolated, rather than using sqlite3's parameter substitution. Even if current callers passed "safe" values, the API surface itself was poisoned. Any future code path that accepted user input—directly or indirectly—would automatically inherit SQL injection capability without any additional vulnerability being introduced.

Exploitation Path

Consider the Update() method. The parameters columnvalue and newcolumnvalue were string fragments like "pid=123" that were spliced into the WHERE and SET clauses. An attacker controlling either parameter could inject arbitrary SQL:

newcolumnvalue = 'ucoind=100; DROP TABLE info; --'

This would produce:

UPDATE "main"."info" SET ucoind=100; DROP TABLE info; -- WHERE pid=123

The SQLite database—containing user identifiers, currency balances (ucoin, ucoind), and platform IDs (pid)—could be read, modified, or destroyed.

The Fix

The fix replaces f-string construction with sqlite3's parameterized query API. This is not a "sanitization" approach—it's a structural change that separates code from data entirely.

Before (Insert):

def Insert(self, column, value):
    sql = f'''INSERT INTO "main"."info" ({column}) VALUES ({value})'''
    self.cursor.execute(sql)

After (Insert):

def Insert(self, value):
    sql = 'INSERT INTO "main"."info" ("pid", "useractualid", "userid", "ucoin", "ucoind") VALUES (?, ?, ?, ?, ?)'
    self.cursor.execute(sql, value)

Before (Update):

def Update(self, columnvalue, newcolumnvalue):
    sql = f'UPDATE "main"."info" SET {newcolumnvalue} WHERE {columnvalue}'
    self.cursor.execute(sql)

After (Update):

def Update(self, pid, ucoind):
    sql = 'UPDATE "main"."info" SET "ucoind"=? WHERE "pid"=?'
    self.cursor.execute(sql, (ucoind, pid))

The changes are deliberate and API-breaking:

1. Fixed schema: Insert() no longer accepts arbitrary columns. The five-column schema is hardcoded, preventing column-name injection.
2. Tuple parameters: Values are passed as a second argument to execute(), not concatenated into the SQL string.
3. Explicit parameters: Update() now takes pid and ucoind directly, eliminating the string-fragment pattern entirely.

Defense in Depth: XML Parser Hardening

The same commit includes an additional security improvement: the GetUID() function, which parses HTML from an external forum, now uses a hardened XML parser configuration:

xml_parser = etree.HTMLParser(resolve_entities=False, no_network=True)
html = etree.HTML(_html1, parser=xml_parser)

This prevents XML External Entity (XXE) attacks and blocks the parser from making network requests during document parsing—relevant when processing untrusted HTML from forum topics.

Key Takeaways

• F-strings in SQL are always wrong: Python's f-string formatting has no place in database query construction. The sqlite3 module's parameter substitution (? placeholders with tuple arguments) is the only correct pattern.
• API design matters for security: The original Insert(column, value) and Update(columnvalue, newcolumnvalue) signatures encouraged callers to build SQL fragments. The fixed signatures force callers to provide data values, not code.
• "Currently safe" is not "safe": Even if audit showed no external input reached these methods, the vulnerable pattern created persistent risk. Code changes, refactoring, or new features could expose the injection surface without anyone recognizing the danger.
• Log what you execute, not what you build: The logger.debug(sql) call in the original code logged the fully-interpolated string—potentially exposing sensitive data in logs. The fixed version logs the parameterized query template.
• Harden parsers processing external data: The resolve_entities=False, no_network=True configuration for etree.HTMLParser demonstrates defense-in-depth for any code handling untrusted markup.

How Orbis AppSec Detected This

Source: The value parameter passed to Insert() and the ucoind/pid parameters passed to Update(), which could originate from any caller of the SQL class methods.

Sink: The cursor.execute() method invoked with a single string argument containing f-string-interpolated SQL, specifically the patterns self.cursor.execute(sql) where sql was constructed via f-string formatting.

Missing control: No use of sqlite3 parameterized queries; the code passed fully-constructed SQL strings rather than using ? placeholders with separate parameter tuples. No validation that column, value, columnvalue, or newcolumnvalue contained only expected characters.

CWE: CWE-89 — Improper Neutralization of Special Elements used in an SQL Command ('SQL Injection')

Fix: Replaced f-string SQL construction with parameterized queries using sqlite3's ? placeholders and tuple parameters, and hardened the etree.HTMLParser configuration to prevent XXE.

Orbis AppSec automatically detected this vulnerability and opened a pull request with the fix. Try Orbis AppSec on your repositories to find and fix issues like this automatically.

Conclusion

This fix demonstrates that SQL injection remains a critical risk even in modern Python code using standard libraries correctly—if those libraries are misused. The sqlite3 module provides robust protection against injection, but only when developers use parameterized queries rather than string formatting. The API redesign of Insert() and Update() shows how security can be built into method signatures, making dangerous patterns structurally impossible. For any codebase maintaining database wrapper classes, this pattern of hardcoded schemas with parameterized execution should be the mandatory standard.