Repository navigation
fix(web): validate service and namespace in profile download handler - #15726
Open
musthaf-bughunter wants to merge 1 commit into
Open
musthaf-bughunter wants to merge 1 commit into
musthaf-bughunter wants to merge 1 commit into
Conversation
The dashboard's `GET /profiles/new` handler passes the `service` and
`namespace` query parameters straight into the ServiceProfile
text/template and into an unquoted `Content-Disposition` filename. A
link such as
/profiles/new?namespace=default&service=x%3B%20filename*%3DUTF-8''run.sh%0A---%0AapiVersion:...%0Akind:%20ClusterRoleBinding...
returns a download whose body contains attacker-supplied YAML documents
(for example a ClusterRoleBinding to cluster-admin) next to the
ServiceProfile, and whose `filename*` parameter overrides the intended
`<service>-profile.yml` name. The dashboard only checks the Host header,
so a top-level navigation to the port-forwarded dashboard is enough to
trigger the download.
`linkerd profile --template` already rejects these values by requiring
the service to be a DNS-1035 label and the namespace a DNS-1123 label
before calling the same renderer. Apply the same checks in
handleProfileDownload and return 400 when they fail.
Validation: added TestHandleConfigDownloadInvalidParams, which fails on
main (200 with the injected header and body) and passes with this
change. `go test ./web/...` and `go vet ./web/...` pass.
Signed-off-by: mushtaf saheb <musthaf@medios.digiscrypt.com>
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.
The dashboard's
GET /profiles/newhandler passes theserviceandnamespacequery parameters straight into the ServiceProfiletext/template and into an unquoted
Content-Dispositionfilename. Alink such as
returns a download whose body contains attacker-supplied YAML documents
(for example a ClusterRoleBinding to cluster-admin) next to the
ServiceProfile, and whose
filename*parameter overrides the intended<service>-profile.ymlname. The dashboard only checks the Host header,so a top-level navigation to the port-forwarded dashboard is enough to
trigger the download.
linkerd profile --templatealready rejects these values by requiringthe service to be a DNS-1035 label and the namespace a DNS-1123 label
before calling the same renderer. Apply the same checks in
handleProfileDownload and return 400 when they fail.
Validation: added TestHandleConfigDownloadInvalidParams, which fails on
main (200 with the injected header and body) and passes with this
change.
go test ./web/...andgo vet ./web/...pass.Signed-off-by: mushtaf saheb musthaf@medios.digiscrypt.com