From c543134753a0c5d22881c12404025724cb05ffd8 Mon Sep 17 00:00:00 2001 From: Jack Christensen Date: Mon, 4 Mar 2024 09:05:32 -0600 Subject: [PATCH] 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. --- internal/sanitize/sanitize.go | 4 ++++ internal/sanitize/sanitize_test.go | 28 +++++++++++++++++++--------- 2 files changed, 23 insertions(+), 9 deletions(-) diff --git a/internal/sanitize/sanitize.go b/internal/sanitize/sanitize.go index f9091cd48..08d24fe47 100644 --- 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 index e2533aabd..191bf1e95 100644 --- 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)`, }, }