Go ArticleRemove() SQL Injection via fmt.Sprintf IN Clause

TITLE: Go ArticleRemove() SQL Injection via fmt.Sprintf IN Clause

SEO_TITLE: ArticleRemove() SQL Injection: Parameterized Fix

SEO_DESCRIPTION: ArticleRemove()'s fmt.Sprintf-built DELETE query let attacker-controlled IDs inject SQL; fixed with parameterized placeholders. CWE-89.

SUMMARY: The Go backend's ArticleRemove function built a SQL DELETE statement by joining a caller-supplied slice of article IDs directly into the query string with fmt.Sprintf. Because the aids values came straight from the frontend with no validation or escaping, an attacker could inject arbitrary SQL into the IN(...) clause. The fix replaces string concatenation with a parameterized query using placeholders and a DB.Exec argument list.

INTRODUCTION: This SQL injection let an attacker who controls the aids parameter sent to ArticleRemove rewrite the executed SQL statement, turning a bulk-delete feature into an arbitrary query execution primitive. The vulnerable function took a []string of article IDs and built the query with strings.Join(aids[:], ",") fed straight into fmt.Sprintf("delete from t_article where id in(%s)", ...). There was no type check, no escaping, and no parameter binding — each element of aids landed in the final SQL text verbatim. Any backend endpoint that takes a list from the client and hands it to fmt.Sprintf for query construction has this exact shape of bug, which is why it's worth understanding precisely how this one broke and how the fix closes it.

Affected Versions

The Vulnerability Explained

The vulnerable implementation of ArticleRemove looked like this:

func (a *App) ArticleRemove(aids []string) *R {
    _, err := DB.Exec(fmt.Sprintf("delete from t_article where id in(%s)", strings.Join(aids[:], ",")))

Every element of aids is dropped, unescaped, directly into the SQL text via strings.Join. The resulting string is then handed to DB.Exec as a complete, pre-built statement — DB.Exec has no opportunity to treat any part of it as data rather than code.

Because aids is attacker-controlled (it arrives from the frontend as the list of article IDs to delete), a single malicious element breaks out of the intended numeric list. Instead of sending ["1","2","3"], an attacker could send something like ["1); DROP TABLE t_article; --"] or, more realistically against most SQL engines, append a subquery or OR 1=1 style predicate inside the IN(...) clause to delete rows the caller was never authorized to touch, or chain further statements depending on the driver's support for multi-statement execution.

The real-world impact for a service exposing this endpoint is severe: an attacker can delete arbitrary rows beyond the ones they're permitted to remove, probe the database structure through error-based or boolean-based injection, or in drivers that permit stacked queries, execute entirely unrelated SQL. This is a classic case where even though the field is "just a list of IDs," skipping parameterization turns a maintenance endpoint into a full SQL injection sink.

The Fix

The fix replaces manual string interpolation with a parameterized query. Instead of building the IN(...) clause out of the raw ID values, it builds a clause out of ? placeholders and passes the actual values as a separate argument slice to DB.Exec:

placeholders := strings.TrimSuffix(strings.Repeat("?,", len(aids)), ",")
args := make([]interface{}, len(aids))
for i, aid := range aids {
    args[i] = aid
}
_, err := DB.Exec(fmt.Sprintf("delete from t_article where id in(%s)", placeholders), args...)

The key difference: fmt.Sprintf is now only used to generate the shape of the query (the correct number of ? placeholders for len(aids)), never to inject actual data values into the SQL text. The real aid values are passed to DB.Exec as args..., which the database driver binds as parameters. The driver — not string concatenation — is responsible for correctly escaping and typing each value before it ever reaches the SQL parser, which eliminates the injection vector regardless of what characters an attacker puts in an ID string.

Key Takeaways

• fmt.Sprintf building a SQL IN(...) clause from a slice is a SQL injection sink even if the values "look like" simple IDs — attackers choose the content of each slice element.
• strings.Join(aids[:], ",") provides no escaping whatsoever; it's a plain string concatenation operation, not a sanitizer.
• The fix pattern — generate ? placeholders with strings.Repeat/strings.TrimSuffix and pass real values via args ...interface{} to DB.Exec — is the correct way to handle variable-length IN clauses in Go's database/sql.
• Any function signature that accepts a []string from the frontend and feeds it into query construction needs the same audit, even when the field name (aids, "article IDs") suggests it's purely numeric.

How Orbis AppSec Detected This

• Source: the aids []string parameter of ArticleRemove, populated directly from frontend request data.
• Sink: DB.Exec(fmt.Sprintf("delete from t_article where id in(%s)", strings.Join(aids[:], ","))).
• Missing control: no parameter binding and no validation that each element of aids was a well-formed numeric ID before being joined into the query string.
• CWE: CWE-89 (SQL Injection).
• Fix: replace the interpolated IN(...) clause with ? placeholders and pass the ID values through DB.Exec's variadic argument list.

Orbis AppSec detected this vulnerability automatically. Try Orbis AppSec on your repositories to find and fix issues like this.

Conclusion

ArticleRemove's bug is a reminder that fmt.Sprintf and SQL text don't mix, even for fields that look like simple ID lists — the moment a slice length and its contents both come from the client, string-joining it into a query hands the attacker control over the statement itself. The fix keeps the dynamic placeholder generation (since database/sql has no native variadic IN syntax) but moves every actual value into the parameter list, letting the driver handle escaping instead of hand-rolled string concatenation.

TAGS: sql-injection, go, database-sql, parameterized-queries, backend-security

FAQ:
Q: Does the fix for ArticleRemove change its public function signature?
A: No, it still takes aids []string and returns *R; only the internal query construction changed from string interpolation to placeholder binding with DB.Exec.

Q: Why does the fixed code still use fmt.Sprintf at all?
A: It uses fmt.Sprintf only to generate the correct number of ? placeholders for the IN(...) clause based on len(aids) — the actual ID values are passed separately as args..., never concatenated into the query text.

Q: What specifically made strings.Join(aids[:], ",") exploitable in the original code?
A: It inserted every element of the attacker-controlled aids slice directly into the SQL string with no escaping or type checking, so any element containing SQL syntax became part of the executed statement.