chore(architecture): re-structure files and folders #63
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| JSON.parse(contentData) | ||
| // If valid, clear content and set as attribute | ||
| this.clearContent() | ||
| this.setAttribute("data", contentData) |
Check warning
Code scanning / CodeQL
DOM text reinterpreted as HTML Medium
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 3 months ago
The root of the problem is that when getContentData() fails to parse the content as JSON, it returns the raw element text, which may contain characters that pose a risk (e.g., <, >, &). This raw value is then persisted as the data attribute; although this may not by itself cause XSS, if the attribute is ever inserted as HTML elsewhere it could.
The correct fix is: Escape and/or validate any non-JSON fallback text before storing it or, even safer, do not assign invalid JSON to the data attribute at all.
The best solution is to avoid storing invalid/untrusted raw text in the data attribute. Instead, if parsing fails, simply leave the attribute unset and treat the error via logging/display.
Alternatively, if you must store the content, sanitize/escape the output before assigning, e.g., replacing <, >, &, ", and ' with entity equivalents.
Steps to fix:
- In the getter for
data, on fallback (when JSON parsing fails), do not assign the raw content to thedataattribute; instead, only assigndataif and only if the text is valid JSON. - Optionally, if you want to store some representation of the invalid content, escape it first.
Files/regions to change:
- In
src/package/components/json-viewer.ts, update lines 239–242: only set thedataattribute if parsing succeeds; otherwise, do not store the untrusted text.
No additional imports or dependencies are necessary; no library functions required.
| @@ -243,7 +243,8 @@ | ||
| return contentData | ||
| } catch (error) { | ||
| console.warn("Invalid JSON in element content:", error) | ||
| // Return the content anyway, let the parser handle the error | ||
| // Return the content anyway, let the parser handle the error, | ||
| // but do NOT set it as data attribute (avoid unsafe fallback persistence) | ||
| return contentData | ||
| } | ||
| } |
| highlightedText.substring(end) | ||
| }) | ||
|
|
||
| el.innerHTML = highlightedText |
Check warning
Code scanning / CodeQL
DOM text reinterpreted as HTML Medium
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 3 months ago
How to fix:
Always ensure that, when mixing user- or DOM-sourced text with inserted markup (such as highlighting), all text segments are individually escaped before inserting them into innerHTML. A robust way is to never build the highlighted markup via substring manipulation and concatenation, but instead:
- Use the search result to split
originalTextinto text and match fragments. - Escape each fragment (both matches and non-matches).
- Concatenate the escaped fragments with highlighted span markup surrounding each match.
This method ensures no unescaped DOM content is ever inserted as HTML.
Precise change needed:
- In the
searchfunction, replace the logic in lines 231–243 for constructinghighlightedText. - Instead of naïvely concatenating substrings possibly containing html, build
highlightedTextby iterating throughoriginalTextand performing escaping for every segment (match and non-match), so all DOM-sourced segments are escaped.
What's needed:
- No new imports.
- A helper code block in
search()to buildhighlightedTextsecurely.
| @@ -228,17 +228,20 @@ | ||
|
|
||
| if (matches.length > 0) { | ||
| found = true | ||
| let highlightedText = originalText | ||
|
|
||
| // Replace matches with highlighted version (reverse order to maintain indices) | ||
| matches.reverse().forEach((match) => { | ||
| // Safely build highlightedText by escaping all fragments | ||
| let lastIndex = 0 | ||
| let highlightedText = "" | ||
| matches.forEach((match) => { | ||
| const start = match.index! | ||
| const end = start + match[0].length | ||
| highlightedText = | ||
| highlightedText.substring(0, start) + | ||
| `<span class="match">${this.escapeHtml(match[0])}</span>` + | ||
| highlightedText.substring(end) | ||
| // Escape and append non-match fragment | ||
| highlightedText += this.escapeHtml(originalText.substring(lastIndex, start)) | ||
| // Escape and append matched fragment, wrapped | ||
| highlightedText += `<span class="match">${this.escapeHtml(match[0])}</span>` | ||
| lastIndex = end | ||
| }) | ||
| // Append and escape any remaining text after last match | ||
| highlightedText += this.escapeHtml(originalText.substring(lastIndex)) | ||
|
|
||
| el.innerHTML = highlightedText | ||
| } |
No description provided.