SQL sanitizer wraps arguments in parentheses

pgx v5 was not vulnerable to CVE-2024-27289 do to how the sanitizer was being called. But the sanitizer itself still had the underlying issue. This commit ports the fix from pgx v4 to v5 to ensure that the issue does not emerge if pgx uses the sanitizer differently in the future.
jackc · Mar 4, 2024 · c543134 · c543134
1 parent 20344df
commit c543134
Show file tree

Hide file tree

Showing 2 changed files with 23 additions and 9 deletions.
diff --git a/internal/sanitize/sanitize.go b/internal/sanitize/sanitize.go
@@ -63,6 +63,10 @@ func (q *Query) Sanitize(args ...any) (string, error) {
 				return "", fmt.Errorf("invalid arg type: %T", arg)
 			}
 			argUse[argIdx] = true
+
+			// Prevent SQL injection via Line Comment Creation
+			// https://github.com/jackc/pgx/security/advisories/GHSA-m7wr-2xf7-cm9p
+			str = "(" + str + ")"
 		default:
 			return "", fmt.Errorf("invalid Part type: %T", part)
 		}

diff --git a/internal/sanitize/sanitize_test.go b/internal/sanitize/sanitize_test.go
@@ -132,47 +132,57 @@ func TestQuerySanitize(t *testing.T) {
 		{
 			query:    sanitize.Query{Parts: []sanitize.Part{"select ", 1}},
 			args:     []any{int64(42)},
-			expected: `select 42`,
+			expected: `select (42)`,
 		},
 		{
 			query:    sanitize.Query{Parts: []sanitize.Part{"select ", 1}},
 			args:     []any{float64(1.23)},
-			expected: `select 1.23`,
+			expected: `select (1.23)`,
 		},
 		{
 			query:    sanitize.Query{Parts: []sanitize.Part{"select ", 1}},
 			args:     []any{true},
-			expected: `select true`,
+			expected: `select (true)`,
 		},
 		{
 			query:    sanitize.Query{Parts: []sanitize.Part{"select ", 1}},
 			args:     []any{[]byte{0, 1, 2, 3, 255}},
-			expected: `select '\x00010203ff'`,
+			expected: `select ('\x00010203ff')`,
 		},
 		{
 			query:    sanitize.Query{Parts: []sanitize.Part{"select ", 1}},
 			args:     []any{nil},
-			expected: `select null`,
+			expected: `select (null)`,
 		},
 		{
 			query:    sanitize.Query{Parts: []sanitize.Part{"select ", 1}},
 			args:     []any{"foobar"},
-			expected: `select 'foobar'`,
+			expected: `select ('foobar')`,
 		},
 		{
 			query:    sanitize.Query{Parts: []sanitize.Part{"select ", 1}},
 			args:     []any{"foo'bar"},
-			expected: `select 'foo''bar'`,
+			expected: `select ('foo''bar')`,
 		},
 		{
 			query:    sanitize.Query{Parts: []sanitize.Part{"select ", 1}},
 			args:     []any{`foo\'bar`},
-			expected: `select 'foo\''bar'`,
+			expected: `select ('foo\''bar')`,
 		},
 		{
 			query:    sanitize.Query{Parts: []sanitize.Part{"insert ", 1}},
 			args:     []any{time.Date(2020, time.March, 1, 23, 59, 59, 999999999, time.UTC)},
-			expected: `insert '2020-03-01 23:59:59.999999Z'`,
+			expected: `insert ('2020-03-01 23:59:59.999999Z')`,
+		},
+		{
+			query:    sanitize.Query{Parts: []sanitize.Part{"select 1-", 1}},
+			args:     []any{int64(-1)},
+			expected: `select 1-(-1)`,
+		},
+		{
+			query:    sanitize.Query{Parts: []sanitize.Part{"select 1-", 1}},
+			args:     []any{float64(-1)},
+			expected: `select 1-(-1)`,
 		},
 	}