How SQL Injection happens in Node.js MySQL queries and how to fix it

Introduction

In the divisible_asset.js file, which handles validation and storage of divisible private payments, a critical SQL injection vulnerability was lurking in the validateAndSaveDivisiblePrivatePayment function. The flaw existed around line 50, where a SQL query was being constructed to look up addresses from the outputs table.

The vulnerable code built an address_sql subquery by directly concatenating src_message_index and src_output_index variables into the SQL string—variables that ultimately originate from external payment element data. While src_unit and arrAuthorAddresses were properly escaped using conn.escape(), these two numeric-looking parameters slipped through without protection.

This matters significantly because this is a Node.js library consumed by downstream applications. Any application using this package to process private payments inherits this vulnerability, potentially exposing their databases to injection attacks through maliciously crafted payment data.

The Vulnerability Explained

SQL injection occurs when untrusted data is incorporated into SQL queries without proper sanitization or parameterization. In this case, the vulnerable code looked like this:

address_sql = "(SELECT address FROM outputs \
    WHERE unit="+conn.escape(src_unit)+" AND message_index="+src_message_index+" \
        AND output_index="+src_output_index+" AND address IN("+conn.escape(arrAuthorAddresses)+"))";

Notice the inconsistency: src_unit and arrAuthorAddresses are wrapped in conn.escape(), but src_message_index and src_output_index are concatenated directly. This creates a dangerous assumption—that these values will always be safe integers.

The Attack Vector

An attacker could craft a malicious private payment element where input.message_index or input.output_index contains SQL injection payloads instead of expected numeric values. For example:

// Malicious payment element
{
    input: {
        message_index: "1; DROP TABLE outputs; --",
        output_index: "0"
    }
}

When this malicious data flows into the validateAndSaveDivisiblePrivatePayment function, the resulting SQL query would become:

SELECT address FROM outputs 
WHERE unit='abc123' AND message_index=1; DROP TABLE outputs; -- AND output_index=0

This could allow an attacker to:
- Extract sensitive data using UNION-based injection
- Modify or delete records with UPDATE/DELETE statements
- Bypass authentication logic in subsequent queries
- Execute administrative operations depending on database permissions

The impact is severe because this code handles financial transactions. An attacker could potentially manipulate payment validation, redirect funds, or corrupt the entire payment history.

The Fix

The fix is elegantly simple—apply the same conn.escape() treatment to src_message_index and src_output_index that was already being used for other parameters:

Before (Vulnerable)

address_sql = "(SELECT address FROM outputs \
    WHERE unit="+conn.escape(src_unit)+" AND message_index="+src_message_index+" \
        AND output_index="+src_output_index+" AND address IN("+conn.escape(arrAuthorAddresses)+"))";

After (Fixed)

address_sql = "(SELECT address FROM outputs \
    WHERE unit="+conn.escape(src_unit)+" AND message_index="+conn.escape(src_message_index)+" \
        AND output_index="+conn.escape(src_output_index)+" AND address IN("+conn.escape(arrAuthorAddresses)+"))";

The conn.escape() function properly sanitizes input by:
1. Converting values to their safe SQL representation
2. Escaping special characters like quotes and backslashes
3. Wrapping strings in quotes appropriately
4. Handling NULL and numeric values correctly

This change ensures that even if an attacker provides malicious strings for message_index or output_index, they will be treated as literal string values rather than executable SQL code. The query will either fail safely (if the values don't match expected data) or execute with the escaped values as harmless strings.

Key Takeaways

• Never assume numeric fields are safe: The message_index and output_index fields appeared to be integers but could contain arbitrary strings from external payment data
• Consistency is crucial: The validateAndSaveDivisiblePrivatePayment function correctly escaped some parameters but missed others—partial protection is no protection
• Library vulnerabilities cascade: Since divisible_asset.js is part of a Node.js library, every downstream consumer inherits this SQL injection risk
• The conn.escape() method was already available: The fix didn't require new dependencies or major refactoring—just consistent application of existing security controls
• Financial transaction code requires extra scrutiny: Payment processing logic is high-value target for attackers and deserves thorough security review

How Orbis AppSec Detected This

• Source: External payment data flowing through input.message_index and input.output_index fields in private payment elements
• Sink: SQL query construction via string concatenation in divisible_asset.js:50-53
• Missing control: No escaping or parameterization for src_message_index and src_output_index variables before SQL query concatenation
• CWE: CWE-89 (Improper Neutralization of Special Elements used in an SQL Command)
• Fix: Applied conn.escape() to both src_message_index and src_output_index parameters to neutralize potential SQL injection payloads

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 SQL injection vulnerability in divisible_asset.js demonstrates how easily security gaps can emerge from inconsistent coding practices. Two parameters escaped, two parameters forgotten—that's all it takes to create a critical vulnerability in financial transaction processing code.

The fix was straightforward: apply conn.escape() consistently to all user-controlled values. But the lesson runs deeper. When building SQL queries, treat every external value as potentially malicious, regardless of whether it "should" be a number or appears to come from a trusted source. Attackers don't play by the rules your code expects.

For Node.js developers working with MySQL, make parameterized queries or consistent escaping a non-negotiable habit. Your database—and your users' data—depends on it.