Skip to content

Commit 75cefa8

Browse files
jkowallgoogle-labs-jules[bot]yurishkuroCopilot
authored
🔒 [security fix] Replace innerHTML with textContent to mitigate XSS risk (#3651)
🎯 **What:** The vulnerability fixed is a potential Cross-Site Scripting (XSS) issue in the `calcRects` function of `packages/jaeger-ui/src/components/DeepDependencies/Graph/DdgNodeContent/calc-positioning.ts`. ⚠️ **Risk:** The use of `innerHTML` when setting the content of a measurement span could allow an attacker to inject and execute malicious scripts if the input strings (like service or operation names) are not properly sanitized or originate from an untrusted source. 🛡️ **Solution:** The fix replaces `innerHTML` with `textContent`. Since the code is only intended to measure the visual dimensions of plain text strings, `textContent` provides a secure alternative that prevents the browser from parsing the input as HTML, thus eliminating the XSS vector. A security-focused unit test has also been added to verify this behavior. --- *PR created automatically by Jules for task [10016137243870737156](https://jules.google.com/task/10016137243870737156) started by @jkowall* --------- Signed-off-by: Yuri Shkuro <yurishkuro@users.noreply.github.com> Signed-off-by: Jonah Kowall <jkowall@kowall.net> Co-authored-by: google-labs-jules[bot] <161369871+google-labs-jules[bot]@users.noreply.github.com> Co-authored-by: jkowall <1859948+jkowall@users.noreply.github.com> Co-authored-by: Yuri Shkuro <yurishkuro@users.noreply.github.com> Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
1 parent c39f6ff commit 75cefa8

File tree

2 files changed

+31
-3
lines changed

2 files changed

+31
-3
lines changed

packages/jaeger-ui/src/components/DeepDependencies/Graph/DdgNodeContent/calc-positioning.test.js

Lines changed: 29 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
// SPDX-License-Identifier: Apache-2.0
33

44
import calcPositioning, { _initSvcSpan, _initOpSpan } from './calc-positioning';
5-
import { FONT_SIZE, LINE_HEIGHT, OP_PADDING_TOP } from './constants';
5+
import { FONT_SIZE, LINE_HEIGHT, OP_PADDING_TOP, WORD_RX } from './constants';
66

77
describe('initializing measuring spans', () => {
88
afterEach(() => {
@@ -227,4 +227,32 @@ describe('calcPositioning', () => {
227227
expect(measureOp).not.toHaveBeenCalled();
228228
});
229229
});
230+
231+
describe('security', () => {
232+
it('treats input strings containing HTML tags as plain text', () => {
233+
const svcSpan = _initSvcSpan();
234+
const xssString = '<img src=x onerror=alert(1)>';
235+
const wordCount = xssString.match(WORD_RX)?.length || 1;
236+
const capturedHtml = [];
237+
const originalImplementation = measureSvc.getMockImplementation();
238+
svcMeasurements = genWidths(new Array(wordCount).fill(1));
239+
240+
measureSvc.mockImplementation(() => {
241+
capturedHtml.push(svcSpan.innerHTML);
242+
return originalImplementation();
243+
});
244+
245+
try {
246+
calcPositioning(xssString);
247+
} finally {
248+
measureSvc.mockImplementation(originalImplementation);
249+
}
250+
251+
expect(capturedHtml).toHaveLength(wordCount);
252+
capturedHtml.forEach(html => {
253+
expect(html).not.toContain('<img');
254+
});
255+
expect(capturedHtml.some(html => html.includes('&lt;img'))).toBe(true);
256+
});
257+
});
230258
});

packages/jaeger-ui/src/components/DeepDependencies/Graph/DdgNodeContent/calc-positioning.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -139,9 +139,9 @@ function calcWidth(lengths: number[], lines: number, longestThusFar = 0): number
139139
*/
140140
const calcRects = _memoize(
141141
function calcRects(str: string | string[], span: HTMLSpanElement): TRect[] {
142-
const lengths = (Array.isArray(str) ? [`${str.length} Operations}`] : str.match(WORD_RX) || [str]).map(
142+
const lengths = (Array.isArray(str) ? [`${str.length} Operations`] : str.match(WORD_RX) || [str]).map(
143143
s => {
144-
span.innerHTML = s;
144+
span.textContent = s;
145145
return span.getClientRects()[0].width;
146146
}
147147
);

0 commit comments

Comments
 (0)