-
Notifications
You must be signed in to change notification settings - Fork 14
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Revert "Staging to Production" #1489
Conversation
@@ -9,7 +9,7 @@ | |||
if (phaseWinner.overview_image_path) { | |||
return ( | |||
<img | |||
src={`${imageBase}${encodeURIComponent(phaseWinner.overview_image_path)}`} | |||
src={imageBase + phaseWinner.overview_image_path} |
Check warning
Code scanning / CodeQL
DOM text reinterpreted as HTML Medium
DOM text
DOM text
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix AI about 1 month ago
To fix the problem, we need to ensure that the imageBase
value is properly sanitized before it is used in the src
attribute of the img
tag. This can be achieved by validating that the imageBase
is a well-formed URL and does not contain any malicious content.
- Import a well-known library for URL validation, such as
validator
. - Validate the
imageBase
value before using it in thesrc
attribute. - If the
imageBase
value is not valid, use a default safe value or handle the error appropriately.
-
Copy modified line R4 -
Copy modified line R8 -
Copy modified line R14
@@ -3,2 +3,3 @@ | ||
import { ApiUrlContext } from '../../ApiUrlContext' | ||
import validator from 'validator'; | ||
|
||
@@ -6,2 +7,3 @@ | ||
const { imageBase } = useContext(ApiUrlContext) | ||
const safeImageBase = validator.isURL(imageBase) ? imageBase : ''; | ||
|
||
@@ -11,3 +13,3 @@ | ||
<img | ||
src={imageBase + phaseWinner.overview_image_path} | ||
src={safeImageBase + phaseWinner.overview_image_path} | ||
alt="Phase Winner image" |
-
Copy modified lines R45-R46
@@ -44,3 +44,4 @@ | ||
"uswds": "2.14.0", | ||
"yarn": "^1.22.19" | ||
"yarn": "^1.22.19", | ||
"validator": "^13.12.0" | ||
}, |
Package | Version | Security advisories |
validator (npm) | 13.12.0 | None |
@@ -23,7 +23,7 @@ | |||
const {id, image_path, name, place_title} = winner | |||
return ( | |||
<div key={id} className="d-flex flex-row align-items-center usa-tbm-1rem"> | |||
{image_path && (<img src={`${imageBase}${encodeURIComponent(winner.image_path)}`} alt="winner image" title="winner image" className="phase-winner-image me-3" />)} | |||
{image_path && <img src={imageBase + winner.image_path} alt="winner image" title="winner image" className="phase-winner-image me-3" />} |
Check warning
Code scanning / CodeQL
DOM text reinterpreted as HTML Medium
DOM text
DOM text
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix AI about 1 month ago
To fix the problem, we need to ensure that the imageBase
value is properly sanitized before being used in the src
attribute of the img
tag. This can be achieved by validating that imageBase
is a safe URL. We can use a well-known library like DOMPurify
to sanitize the URL.
- Install the
DOMPurify
library. - Import
DOMPurify
in the relevant file. - Use
DOMPurify
to sanitize theimageBase
value before concatenating it withwinner.image_path
.
-
Copy modified line R4 -
Copy modified line R8 -
Copy modified line R14 -
Copy modified line R28
@@ -3,2 +3,3 @@ | ||
import { ApiUrlContext } from '../../ApiUrlContext' | ||
import DOMPurify from 'dompurify'; | ||
|
||
@@ -6,2 +7,3 @@ | ||
const { imageBase } = useContext(ApiUrlContext) | ||
const sanitizedImageBase = DOMPurify.sanitize(imageBase); | ||
|
||
@@ -11,3 +13,3 @@ | ||
<img | ||
src={imageBase + phaseWinner.overview_image_path} | ||
src={sanitizedImageBase + phaseWinner.overview_image_path} | ||
alt="Phase Winner image" | ||
@@ -25,3 +27,3 @@ | ||
<div key={id} className="d-flex flex-row align-items-center usa-tbm-1rem"> | ||
{image_path && <img src={imageBase + winner.image_path} alt="winner image" title="winner image" className="phase-winner-image me-3" />} | ||
{image_path && <img src={sanitizedImageBase + winner.image_path} alt="winner image" title="winner image" className="phase-winner-image me-3" />} | ||
{name && <p>{name}</p>} |
-
Copy modified lines R45-R46
@@ -44,3 +44,4 @@ | ||
"uswds": "2.14.0", | ||
"yarn": "^1.22.19" | ||
"yarn": "^1.22.19", | ||
"dompurify": "^3.2.3" | ||
}, |
Package | Version | Security advisories |
dompurify (npm) | 3.2.3 | None |
@@ -18,10 +18,10 @@ | |||
if (custom_url_example_text.length > 0) { | |||
if (custom_url_input_value != "") { | |||
challenge_title_slug = title_to_url_slug(custom_url_input_value) | |||
custom_url_example_text.text(challenge_title_slug) | |||
custom_url_example_text.html(challenge_title_slug) |
Check warning
Code scanning / CodeQL
DOM text reinterpreted as HTML Medium
DOM text
DOM text
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix AI about 1 month ago
To fix the problem, we need to ensure that the input values are properly sanitized before being used to set HTML content. Instead of using the html()
method, we should use the text()
method, which sets the text content of the selected elements and automatically escapes any HTML characters.
- Replace the
html()
method with thetext()
method to prevent the input from being interpreted as HTML. - This change should be made on lines 21 and 24 in the
set_custom_url_example
function.
-
Copy modified line R21 -
Copy modified line R24
@@ -20,6 +20,6 @@ | ||
challenge_title_slug = title_to_url_slug(custom_url_input_value) | ||
custom_url_example_text.html(challenge_title_slug) | ||
custom_url_example_text.text(challenge_title_slug) | ||
} else { | ||
challenge_title_slug = title_to_url_slug(challenge_title_input_value) | ||
custom_url_example_text.html(challenge_title_slug) | ||
custom_url_example_text.text(challenge_title_slug) | ||
} |
} else { | ||
challenge_title_slug = title_to_url_slug(challenge_title_input_value) | ||
custom_url_example_text.text(challenge_title_slug) | ||
custom_url_example_text.html(challenge_title_slug) |
Check warning
Code scanning / CodeQL
DOM text reinterpreted as HTML Medium
DOM text
DOM text
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix AI about 1 month ago
To fix the problem, we need to ensure that any user input is properly escaped before being inserted into the DOM. This can be achieved by using text insertion methods that automatically escape HTML, such as text()
instead of html()
. This change will prevent any HTML tags in the user input from being interpreted as actual HTML, thereby mitigating the XSS risk.
We will replace the html()
method with the text()
method on lines 21 and 24 in the set_custom_url_example
function. This ensures that the user input is treated as plain text and not as HTML.
-
Copy modified line R21 -
Copy modified line R24
@@ -20,6 +20,6 @@ | ||
challenge_title_slug = title_to_url_slug(custom_url_input_value) | ||
custom_url_example_text.html(challenge_title_slug) | ||
custom_url_example_text.text(challenge_title_slug) | ||
} else { | ||
challenge_title_slug = title_to_url_slug(challenge_title_input_value) | ||
custom_url_example_text.html(challenge_title_slug) | ||
custom_url_example_text.text(challenge_title_slug) | ||
} |
Reverts #1456