fix: issue 1928

jackc · Mar 6, 2024 · b17f61f · b17f61f
1 parent da6f2c9
commit b17f61f
Show file tree

Hide file tree

Showing 2 changed files with 76 additions and 12 deletions.
diff --git a/internal/sanitize/sanitize.go b/internal/sanitize/sanitize.go
@@ -66,7 +66,24 @@ func (q *Query) Sanitize(args ...any) (string, error) {
 
 			// Prevent SQL injection via Line Comment Creation
 			// https://github.com/jackc/pgx/security/advisories/GHSA-m7wr-2xf7-cm9p
-			str = "(" + str + ")"
+			// If the value starts with a dash then we need to put parens around it
+			if strings.HasPrefix(str, "-") {
+				str = "(" + str + ")"
+			}
+
+			// Convert problematic line breaks to escape form
+			// See: https://www.postgresql.org/docs/9.0/sql-syntax-lexical.html Table 4.1
+			if strings.ContainsAny(str, "\b\f\n\r\t") {
+				str = "E" + str
+				// Escape existing escapes
+				str = strings.ReplaceAll(str, `\`, `\\`)
+				// Convert existing line breaks to escaped
+				str = strings.ReplaceAll(str, "\b", `\b`)
+				str = strings.ReplaceAll(str, "\f", `\f`)
+				str = strings.ReplaceAll(str, "\n", `\n`)
+				str = strings.ReplaceAll(str, "\r", `\r`)
+				str = strings.ReplaceAll(str, "\t", `\t`)
+			}
 		default:
 			return "", fmt.Errorf("invalid Part type: %T", part)
 		}

diff --git a/internal/sanitize/sanitize_test.go b/internal/sanitize/sanitize_test.go
@@ -132,57 +132,104 @@ 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'`,
 		},
+		// https://github.com/advisories/GHSA-m7wr-2xf7-cm9p
 		{
 			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)`,
+			args:     []any{int64(1)},
+			expected: `select 1-1`,
+		},
+		{
+			query:    sanitize.Query{Parts: []sanitize.Part{"select ", 1, " from test where secure=", 2}},
+			args:     []any{"* -- \n from secrets\n --", true},
+			expected: `select E'* -- \n from secrets\n --' from test where secure=true`,
+		},
+		{
+			query:    sanitize.Query{Parts: []sanitize.Part{"select * from example where result=-", 1, " OR name=", 2}},
+			args:     []any{float64(-42), "foo\n 1 AND 1=0 UNION SELECT * FROM secrets; --"},
+			expected: `select * from example where result=-(-42) OR name=E'foo\n 1 AND 1=0 UNION SELECT * FROM secrets; --'`,
+		},
+		{
+			query:    sanitize.Query{Parts: []sanitize.Part{"select * from example where result=-", 1, " OR name=", 2}},
+			args:     []any{"-42", "foo\n 1 AND 1=0 UNION SELECT * FROM secrets; --"},
+			expected: `select * from example where result=-'-42' OR name=E'foo\n 1 AND 1=0 UNION SELECT * FROM secrets; --'`,
+		},
+		{
+			query:    sanitize.Query{Parts: []sanitize.Part{"select ", 1}},
+			args:     []any{"\b\f\n\r\t"},
+			expected: `select E'\b\f\n\r\t'`,
+		},
+		{
+			query:    sanitize.Query{Parts: []sanitize.Part{"select ", 1}},
+			args:     []any{`\b\f\n\r\t`},
+			expected: `select '\b\f\n\r\t'`,
+		},
+		{
+			query:    sanitize.Query{Parts: []sanitize.Part{"select ", 1}},
+			args:     []any{`\b\f\n\r\t` + "\n"},
+			expected: `select E'\\b\\f\\n\\r\\t\n'`,
+		},
+		{
+			query:    sanitize.Query{Parts: []sanitize.Part{"select ", 1}},
+			args:     []any{`\` + "\n"},
+			expected: `select E'\\\n'`,
+		},
+		// https://github.com/jackc/pgx/issues/1928
+		{
+			query:    sanitize.Query{Parts: []sanitize.Part{"set local someMeta to ", 1}},
+			args:     []any{"some metadata"},
+			expected: `set local someMeta to 'some metadata'`,
+		},
+		{
+			query:    sanitize.Query{Parts: []sanitize.Part{"select INTERVAL ", 1}},
+			args:     []any{"1 day"},
+			expected: `select INTERVAL '1 day'`,
 		},
 	}