From 545775266c3f23ee83c4aaa44f8ba2d2e2d5a5e2 Mon Sep 17 00:00:00 2001 From: Yetkin Timocin Date: Fri, 22 Sep 2023 12:08:30 -0700 Subject: [PATCH] Fixing CodeQL Warnings (#6277) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit # Description Fixing code scanning warnings: https://github.com/radius-project/radius/security/code-scanning. ## Type of change - This pull request is a minor refactor, code cleanup, test improvement, or other maintenance task and doesn't change the functionality of Radius (issue link optional). Fixes: #6081 ## Auto-generated summary ### ๐Ÿค– Generated by Copilot at 527e660 ### Summary ๐Ÿ”’๐Ÿ› ๏ธ๐Ÿงช Improved the security of the UCP API and its tests by sanitizing URL paths and escaping HTML in responses. Added a `sanitizeUrl` function in `pkg/ucp/resources/url.go` and used the `html` package in `test/ucp/httpbaseline/baseline.go`. > _We sanitize the URL, we don't trust the path_ > _We escape the body, we avoid the wrath_ > _Of the evil hackers, who lurk in the dark_ > _They can't redirect us, they can't leave their mark_ ### Walkthrough * Sanitize URL path to prevent open redirect vulnerabilities in UCP API ([link](https://github.com/radius-project/radius/pull/6277/files?diff=unified&w=0#diff-205044f118b5c4ccc5d1c4c9ac53023e648e8d9888bb8a25aaf572a57640a132R31-R32), [link](https://github.com/radius-project/radius/pull/6277/files?diff=unified&w=0#diff-205044f118b5c4ccc5d1c4c9ac53023e648e8d9888bb8a25aaf572a57640a132R66-R72)) * Escape request body to prevent XSS vulnerabilities in UCP API testing ([link](https://github.com/radius-project/radius/pull/6277/files?diff=unified&w=0#diff-8ca92e0087efe40d9d3317cd14b0e76816be54a550ad56d578bad1c5533a45ffR23), [link](https://github.com/radius-project/radius/pull/6277/files?diff=unified&w=0#diff-8ca92e0087efe40d9d3317cd14b0e76816be54a550ad56d578bad1c5533a45ffL205-R207)) Co-authored-by: Yetkin Timocin --- pkg/ucp/resources/url_test.go | 8 ++++---- test/ucp/httpbaseline/baseline.go | 7 ++++++- 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/pkg/ucp/resources/url_test.go b/pkg/ucp/resources/url_test.go index 0cdae1d0e8..3773478930 100644 --- a/pkg/ucp/resources/url_test.go +++ b/pkg/ucp/resources/url_test.go @@ -24,11 +24,11 @@ import ( func Test_ExtractPlanesPrefixFromURLPath_Invalid(t *testing.T) { data := []string{ - "planes/radius", // Not long enough - - "planes//foo", // Empty segment - + "planes/radius", // Not long enough + "planes//foo", // Empty segment "/subscriptions/test/anotherone/bar", // missing planes + "//planes/radius", // Empty segment + "/", // Empty segment } for _, datum := range data { planeType, planeName, remainder, err := ExtractPlanesPrefixFromURLPath(datum) diff --git a/test/ucp/httpbaseline/baseline.go b/test/ucp/httpbaseline/baseline.go index cc20dc5ccf..f3a6dde458 100644 --- a/test/ucp/httpbaseline/baseline.go +++ b/test/ucp/httpbaseline/baseline.go @@ -20,6 +20,7 @@ import ( "bytes" "context" "encoding/json" + "html" "io" "net/http" "net/http/httptest" @@ -202,7 +203,11 @@ func (rt *RoundTripper) RoundTrip(req *http.Request) (*http.Response, error) { } w.WriteHeader(w.Code) - _, err = w.WriteString(rt.Request.Body) + + // We need to escape/sanitize the content to prevent XSS attacks. + // Please see: https://github.com/radius-project/radius/security/code-scanning/2 + safeContent := html.EscapeString(rt.Request.Body) + _, err = w.WriteString(safeContent) if err != nil { return nil, err }