-
Notifications
You must be signed in to change notification settings - Fork 46
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 6 months 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.textContentbefore 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 6 months 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
videoSrcin a validation step. - Using the
URLconstructor 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