Skip to content

Commit

Permalink
Fixing CodeQL Warnings (#6277)
Browse files Browse the repository at this point in the history
# 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

<!--
copilot:all
-->
### <samp>🤖 Generated by Copilot at 527e660</samp>

### Summary
🔒🛠️🧪

<!--
1. 🔒 - This emoji represents security, which is the main theme of these
changes. The `sanitizeUrl` function prevents open redirect
vulnerabilities, and the `html` package prevents XSS attacks.
2. 🛠️ - This emoji represents fixing or improving something, which is
what the changes do for the UCP API and the test package. The
`sanitizeUrl` function improves the validation of the URL path, and the
`html` package improves the escaping of the request body.
3. 🧪 - This emoji represents testing, which is the context of the
`httpbaseline` package. The changes affect how the test mock responses
are generated and handled.
-->
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 <[email protected]>
  • Loading branch information
ytimocin and Yetkin Timocin authored Sep 22, 2023
1 parent 1e2fc8a commit 5457752
Show file tree
Hide file tree
Showing 2 changed files with 10 additions and 5 deletions.
8 changes: 4 additions & 4 deletions pkg/ucp/resources/url_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
7 changes: 6 additions & 1 deletion test/ucp/httpbaseline/baseline.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import (
"bytes"
"context"
"encoding/json"
"html"
"io"
"net/http"
"net/http/httptest"
Expand Down Expand Up @@ -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
}
Expand Down

0 comments on commit 5457752

Please sign in to comment.