-
Notifications
You must be signed in to change notification settings - Fork 43
User/grant archibald ms/report 594 #631
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
base: main
Are you sure you want to change the base?
Conversation
container.style.display = 'none'; | ||
const relatedBtn = testRow.querySelector(`.toggle-video[data-video-id="${container.id}"]`); | ||
if (relatedBtn) { | ||
relatedBtn.innerHTML = `<i class="fas fa-play-circle"></i> ${relatedBtn.textContent.trim().replace('Pause Video', '').trim() || 'Play Video'}`; |
Check warning
Code scanning / CodeQL
DOM text reinterpreted as HTML Medium
DOM text
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI about 1 month ago
To fix the issue, the text retrieved from relatedBtn.textContent
should be properly escaped before being interpolated into the HTML string. Escaping ensures that any meta-characters in the text are treated as literal text rather than HTML. A utility function or library like DOMPurify
can be used for this purpose, or a simple manual escaping function can be implemented.
Steps to fix:
- Introduce a function to escape HTML meta-characters (e.g.,
<
,>
,&
,"
). - Use this function to sanitize
relatedBtn.textContent
before interpolating it into the HTML string. - Replace the vulnerable line with the sanitized version.
-
Copy modified lines R1230-R1238 -
Copy modified lines R1268-R1269
@@ -1229,2 +1229,11 @@ | ||
// Event listeners for video toggle | ||
// Utility function to escape HTML meta-characters | ||
function escapeHtml(text) { | ||
return text.replace(/&/g, '&') | ||
.replace(/</g, '<') | ||
.replace(/>/g, '>') | ||
.replace(/"/g, '"') | ||
.replace(/'/g, '''); | ||
} | ||
|
||
document.body.addEventListener('click', function(e) { | ||
@@ -1258,3 +1267,4 @@ | ||
if (relatedBtn) { | ||
relatedBtn.innerHTML = `<i class="fas fa-play-circle"></i> ${relatedBtn.textContent.trim().replace('Pause Video', '').trim() || 'Play Video'}`; | ||
const sanitizedText = escapeHtml(relatedBtn.textContent.trim().replace('Pause Video', '').trim() || 'Play Video'); | ||
relatedBtn.innerHTML = `<i class="fas fa-play-circle"></i> ${sanitizedText}`; | ||
relatedBtn.classList.remove('btn-secondary'); |
// Set source if not already set | ||
const sourceElement = videoElement.querySelector('source'); | ||
if (sourceElement && !sourceElement.src && videoSrc) { | ||
sourceElement.src = videoSrc; |
Check warning
Code scanning / CodeQL
DOM text reinterpreted as HTML Medium
DOM text
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI about 1 month ago
To fix the issue, the value of videoSrc
should be validated or sanitized before being assigned to the src
property of the <source>
element. A simple and effective approach is to ensure that videoSrc
is a valid and safe URL. This can be achieved using the URL
constructor, which throws an error if the input is not a valid URL. Additionally, you can restrict the allowed protocols (e.g., http
and https
) to prevent malicious payloads.
The fix involves:
- Wrapping the assignment of
videoSrc
in a validation step. - Using the
URL
constructor to validate the URL and checking its protocol. - Logging an error and skipping the assignment if the validation fails.
-
Copy modified lines R1279-R1289
@@ -1278,4 +1278,13 @@ | ||
if (sourceElement && !sourceElement.src && videoSrc) { | ||
sourceElement.src = videoSrc; | ||
videoElement.load(); | ||
try { | ||
const validatedUrl = new URL(videoSrc, window.location.origin); | ||
if (validatedUrl.protocol === 'http:' || validatedUrl.protocol === 'https:') { | ||
sourceElement.src = validatedUrl.href; | ||
videoElement.load(); | ||
} else { | ||
console.error('Invalid video source protocol:', validatedUrl.protocol); | ||
} | ||
} catch (err) { | ||
console.error('Invalid video source URL:', videoSrc, err); | ||
} | ||
} |
Extended the Copilot Studio Kit sample to include the following
|
Pull Request Template
Description
Add new test summary option to Test Engine. This feature provides the following:
Checklist