Skip to content

Commit 8710052

Browse files
authored
fix(jsonfilter): validate column identifier before building selector (#3210)
1 parent a2b5c64 commit 8710052

2 files changed

Lines changed: 64 additions & 0 deletions

File tree

pkg/jsonfilter/jsonfilter.go

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,16 @@ import (
3636
// quote) would allow breaking out of the literal and injecting arbitrary SQL.
3737
var fieldPathRegexp = regexp.MustCompile(`^[A-Za-z_][A-Za-z0-9_]*(?:\.[A-Za-z_][A-Za-z0-9_]*|\[[0-9]+\])*$`)
3838

39+
// columnRegexp matches a safe SQL column identifier (a single unqualified
40+
// identifier such as "metadata").
41+
//
42+
// This allowlist is security-critical: the underlying ent sqljson helpers emit
43+
// the column verbatim into the query and, for PostgreSQL, a column containing a
44+
// double quote bypasses ent's identifier quoting entirely and is written raw,
45+
// allowing SQL injection identical to the field-path case. Restricting the
46+
// column to a bare identifier removes every character an injection would need.
47+
var columnRegexp = regexp.MustCompile(`^[A-Za-z_][A-Za-z0-9_]*$`)
48+
3949
// JSONOperator represents supported JSON filter operators.
4050
type JSONOperator string
4151

@@ -66,6 +76,13 @@ func BuildEntSelectorFromJSONFilter(jsonFilter *JSONFilter) (*entsql.Predicate,
6676
return nil, errors.New("invalid filter: column and operator are required")
6777
}
6878

79+
// Validate the column before it reaches the SQL builder. Like the field
80+
// path, the column is emitted into the query by the ent sqljson helpers
81+
// without reliable escaping, so an unsafe value would allow SQL injection.
82+
if err := validateColumn(jsonFilter.Column); err != nil {
83+
return nil, err
84+
}
85+
6986
// Validate the field path before it reaches the SQL builder. The ent
7087
// sqljson helpers concatenate the path segments unescaped into the query,
7188
// so an unsafe value would allow SQL injection.
@@ -105,6 +122,18 @@ func BuildEntSelectorFromJSONFilter(jsonFilter *JSONFilter) (*entsql.Predicate,
105122
}
106123
}
107124

125+
// validateColumn ensures the JSON column is a bare SQL identifier before it is
126+
// emitted into the query by the ent sqljson helpers. Callers are expected to set
127+
// it to a known column constant, but validating here keeps safety from depending
128+
// on every caller remembering to do so.
129+
func validateColumn(column string) error {
130+
if !columnRegexp.MatchString(column) {
131+
return fmt.Errorf("invalid column %q: must be a valid identifier", column)
132+
}
133+
134+
return nil
135+
}
136+
108137
// validateFieldPath ensures the JSON field path only contains safe characters
109138
// before it is concatenated into the SQL query by the ent sqljson helpers.
110139
// An empty path is allowed: it targets the column itself and is not injectable.

pkg/jsonfilter/jsonfilter_test.go

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,9 @@ import (
2525
// errInvalidFieldPath is the common prefix returned when a field path fails validation.
2626
const errInvalidFieldPath = "invalid field path"
2727

28+
// errInvalidColumn is the common prefix returned when a column fails validation.
29+
const errInvalidColumn = "invalid column"
30+
2831
func TestBuildEntSelectorFromJSONFilter(t *testing.T) {
2932
tests := []struct {
3033
name string
@@ -46,6 +49,38 @@ func TestBuildEntSelectorFromJSONFilter(t *testing.T) {
4649
filter: &JSONFilter{Column: "metadata", Operator: "gt", Value: "foo"},
4750
wantErr: "unsupported operator: gt",
4851
},
52+
{
53+
// A double quote in the column breaks out of the identifier quoting
54+
// performed by ent's builder, allowing raw SQL injection.
55+
name: "column with double quote breaks out of identifier",
56+
filter: &JSONFilter{Column: `metadata" OR "1"="1`, FieldPath: "name", Operator: OpEQ, Value: "foo"},
57+
wantErr: errInvalidColumn,
58+
},
59+
{
60+
name: "column with single quote",
61+
filter: &JSONFilter{Column: `metadata' OR '1'='1`, FieldPath: "name", Operator: OpEQ, Value: "foo"},
62+
wantErr: errInvalidColumn,
63+
},
64+
{
65+
name: "column with whitespace",
66+
filter: &JSONFilter{Column: "metadata OR 1=1", FieldPath: "name", Operator: OpEQ, Value: "foo"},
67+
wantErr: errInvalidColumn,
68+
},
69+
{
70+
name: "column with parenthesis",
71+
filter: &JSONFilter{Column: "pg_sleep(2)", FieldPath: "name", Operator: OpEQ, Value: "foo"},
72+
wantErr: errInvalidColumn,
73+
},
74+
{
75+
name: "column starting with digit",
76+
filter: &JSONFilter{Column: "1metadata", FieldPath: "name", Operator: OpEQ, Value: "foo"},
77+
wantErr: errInvalidColumn,
78+
},
79+
{
80+
name: "column with dot qualifier",
81+
filter: &JSONFilter{Column: "workflow.metadata", FieldPath: "name", Operator: OpEQ, Value: "foo"},
82+
wantErr: errInvalidColumn,
83+
},
4984
{
5085
name: "eq operator with string value",
5186
filter: &JSONFilter{Column: "metadata", FieldPath: "name", Operator: OpEQ, Value: "foo"},

0 commit comments

Comments
 (0)