Skip to content

Commit f96392b

Browse files
author
-g
committed
fix(query): reject reserved encoded-query characters in condition values
Condition values were written into the encoded query verbatim, so a value containing ^, ,, or @ could break out of its term and append arbitrary clauses to sysparm_query (encoded-query injection). ServiceNow encoded queries provide no escape sequence for these structural characters, so values are now validated at construction time: a rejected value yields an error Condition that surfaces through the existing Error() channel and propagates via And()/Or(). Internally composed date literals (OnSpecialty, JS, NewDateTimeValue) keep their intentional @ / javascript: separators and bypass validation. Fixes #645
1 parent 97057de commit f96392b

8 files changed

Lines changed: 372 additions & 12 deletions

File tree

query/base_field.go

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,15 +20,32 @@ func (f BaseField) buildBinary(op ast.Operator, right ast.Node) Condition {
2020
}
2121

2222
func (f BaseField) binary(op ast.Operator, val interface{}) Condition {
23+
if err := validateQueryValue(f.name, op, val); err != nil {
24+
return NewErrorCondition(err)
25+
}
2326
return f.buildBinary(op, ast.NewLiteralNode(val))
2427
}
2528

2629
func (f BaseField) pair(op ast.Operator, left, right interface{}) Condition {
30+
if err := validateQueryValue(f.name, op, left); err != nil {
31+
return NewErrorCondition(err)
32+
}
33+
if err := validateQueryValue(f.name, op, right); err != nil {
34+
return NewErrorCondition(err)
35+
}
2736
return f.buildBinary(op, ast.NewPairNode(ast.NewLiteralNode(left), ast.NewLiteralNode(right)))
2837
}
2938

30-
func (f BaseField) multi(op ast.Operator, nodes *ast.ArrayNode) Condition {
31-
return f.buildBinary(op, nodes)
39+
// multi builds a condition over multiple primitive values (e.g., IN / NOT IN),
40+
// validating each value before building the array node. It is a package-level
41+
// function because Go methods cannot declare type parameters.
42+
func multi[T ast.Primitive](f BaseField, op ast.Operator, values ...T) Condition {
43+
for _, value := range values {
44+
if err := validateQueryValue(f.name, op, value); err != nil {
45+
return NewErrorCondition(err)
46+
}
47+
}
48+
return f.buildBinary(op, convertSliceToArrayNode(values...))
3249
}
3350

3451
// IsAnything query that field is anything.

query/date_time_field.go

Lines changed: 14 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -13,18 +13,26 @@ type DateTimeField struct {
1313
}
1414

1515
func (f DateTimeField) dateTimeBinary(op ast.Operator, val any) Condition {
16-
var literal string
1716
switch v := val.(type) {
1817
case DateTimeValue:
19-
literal = v.String()
18+
// Trusted composite built by this package (NewDateTimeValue, Time, JS,
19+
// OnSpecialty): its "@" and "javascript:" separators are intentional
20+
// encoded-query syntax, so it bypasses value validation.
21+
return f.buildBinary(op, ast.NewLiteralNode(v.String()))
2022
case time.Time:
21-
literal = v.Format("2006-01-02 15:04:05")
23+
return f.buildBinary(op, ast.NewLiteralNode(v.Format("2006-01-02 15:04:05")))
2224
case string:
23-
literal = v
25+
if err := validateQueryValue(f.name, op, v); err != nil {
26+
return NewErrorCondition(err)
27+
}
28+
return f.buildBinary(op, ast.NewLiteralNode(v))
2429
default:
25-
literal = fmt.Sprintf("%v", v)
30+
literal := fmt.Sprintf("%v", v)
31+
if err := validateQueryValue(f.name, op, literal); err != nil {
32+
return NewErrorCondition(err)
33+
}
34+
return f.buildBinary(op, ast.NewLiteralNode(literal))
2635
}
27-
return f.binary(op, literal)
2836
}
2937

3038
// On query the date-time field is on a specific date-time.

query/number_field.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -51,12 +51,12 @@ func (f NumberField) Between(lower, upper float64) Condition {
5151

5252
// IsOneOf query that field is one of the provided values.
5353
func (f NumberField) IsOneOf(values ...float64) Condition {
54-
return f.multi(ast.OperatorIsOneOf, convertSliceToArrayNode(values...))
54+
return multi(f.BaseField, ast.OperatorIsOneOf, values...)
5555
}
5656

5757
// IsNotOneOf query that field is not one of the provided values.
5858
func (f NumberField) IsNotOneOf(values ...float64) Condition {
59-
return f.multi(ast.OperatorIsNotOneOf, convertSliceToArrayNode(values...))
59+
return multi(f.BaseField, ast.OperatorIsNotOneOf, values...)
6060
}
6161

6262
// GreaterThanField query that field is greater than the provided field.

query/query.go

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,10 @@
11
// Package query provides a type-safe and fluent API for building ServiceNow encoded queries.
22
// It is a redesign of the query package, focusing on usability and immutability.
3+
//
4+
// Condition values are validated at construction time: a value containing one
5+
// of the reserved encoded-query characters ("^", ",", "@") yields an error
6+
// Condition whose Error() is non-nil, because ServiceNow encoded queries offer
7+
// no way to escape those characters inside a value.
38
package query
49

510
// Where starts a new query on the specified field as a string field.

query/string_field.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -41,12 +41,12 @@ func (f StringField) DoesNotContain(val string) Condition {
4141

4242
// IsOneOf query that field is one of the provided values.
4343
func (f StringField) IsOneOf(values ...string) Condition {
44-
return f.multi(ast.OperatorIsOneOf, convertSliceToArrayNode(values...))
44+
return multi(f.BaseField, ast.OperatorIsOneOf, values...)
4545
}
4646

4747
// IsNotOneOf query that field is not one of the provided values.
4848
func (f StringField) IsNotOneOf(values ...string) Condition {
49-
return f.multi(ast.OperatorIsNotOneOf, convertSliceToArrayNode(values...))
49+
return multi(f.BaseField, ast.OperatorIsNotOneOf, values...)
5050
}
5151

5252
// IsEmptyString query that string field is empty.

query/validation.go

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
package query
2+
3+
import (
4+
"fmt"
5+
"strings"
6+
7+
"github.com/michaeldcanady/servicenow-sdk-go/v2/internal/ast"
8+
)
9+
10+
// reservedQueryCharacters lists the ServiceNow encoded-query metacharacters that
11+
// cannot appear inside a condition value: "^" separates clauses ("^" / "^OR"),
12+
// "," separates values in a list, and "@" separates the halves of a pair.
13+
// Encoded queries provide no escape sequence for them, so a value containing
14+
// any of these characters would break out of its term and append arbitrary
15+
// clauses to the query.
16+
const reservedQueryCharacters = "^,@" //nolint:gochecknoglobals
17+
18+
// validateQueryValue checks that a consumer-supplied value contains no reserved
19+
// encoded-query characters. It returns nil for safe values and a descriptive
20+
// error otherwise; callers turn that error into an error Condition via
21+
// NewErrorCondition so it surfaces through Condition.Error().
22+
//
23+
// Literals composed internally by this package (for example the
24+
// "label@javascript:expr@javascript:expr" strings built by OnSpecialty) are
25+
// trusted and must bypass this check.
26+
func validateQueryValue(field string, op ast.Operator, val any) error {
27+
rendered := fmt.Sprintf("%v", val)
28+
index := strings.IndexAny(rendered, reservedQueryCharacters)
29+
if index < 0 {
30+
return nil
31+
}
32+
33+
return fmt.Errorf(
34+
"value %q for field %q (operator %q) contains reserved encoded-query character %q; ^, ,, and @ structure the query itself and cannot be escaped",
35+
rendered, field, op.String(), rendered[index],
36+
)
37+
}

0 commit comments

Comments
 (0)