🛡️ Sentinel: [HIGH] Fix XSS vulnerability in JsonParser#1767
🛡️ Sentinel: [HIGH] Fix XSS vulnerability in JsonParser#1767google-labs-jules[bot] wants to merge 3 commits intodevelopunipept/unipept:developfrom sentinel-fix-xss-jsonparser-15725120700481537122unipept/unipept:sentinel-fix-xss-jsonparser-15725120700481537122Copy head branch name to clipboard
Conversation
The `JsonParser` class was using `JSON.stringify` and regex replacement to generate HTML for syntax highlighting, but it failed to escape HTML special characters in the string values or object keys. This could allow XSS attacks if the input object contained malicious strings (e.g. `<script>`) and the result was rendered via `v-html`. This commit: - Adds a private `escapeHtml` method to `JsonParser`. - Updates `parseAndHighlight` to escape both object keys (after stripping quotes) and string values. - Updates unit tests to expect escaped HTML entities (e.g. `"`). - Adds a specific test case to verify that malicious scripts are escaped.
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
|
Instead of trying to sanitize the generated HTML from JSON ourselves, I think it might be better if we use the dompurify dependency to do the job for us. This dependency is already present in this project, can you adjust? |
Thank you for the suggestion. I have updated the implementation to use |
Replaced the custom regex-based HTML escaping with `dompurify` as requested. This ensures more robust sanitization of the generated HTML. Changes: - Import `dompurify` in `JsonParser.ts`. - Wrap the generated HTML string in `DOMPurify.sanitize()`, allowing `span` tags and `style` attributes. - Update `JsonParser.spec.ts` to use `jsdom` environment (required for DOMPurify) and update test expectations to reflect that malicious tags are stripped rather than escaped.
|
Is it really required to add the |
You are right, we can avoid adding |
- Use `dompurify` to sanitize generated HTML. - Mock `dompurify` in unit tests to avoid `jsdom` dependency. - Remove `jsdom` from devDependencies. Co-authored-by: pverscha <9608686+pverscha@users.noreply.github.com>
I have identified and fixed a stored XSS vulnerability in the
JsonParserclass, which is used to syntax-highlight JSON objects in the UI.Vulnerability
The
JsonParser.parseAndHighlightmethod was constructing HTML strings from JSON objects without escaping HTML special characters in string values or keys. If an attacker could inject malicious content (e.g.,<script>alert(1)</script>) into data that is subsequently displayed using this parser and rendered withv-html, the script would execute.Fix
I modified
src/logic/parsers/json/JsonParser.tsto:escapeHtmlthat escapes&,<,>,", and'.<span>tags.Verification
I updated
src/logic/parsers/json/JsonParser.spec.tsto:"test"instead of"test").<img src=x onerror=alert(1)>) is properly escaped and does not appear raw in the output.All tests passed.
yarn lintalso passed.PR created automatically by Jules for task 15725120700481537122 started by @pverscha