Skip to content

Commit de2e504

Browse files
Use Reflect for script.src (#453)
* Use reflect for script.src as some scripts bind this to call getAttribute causing a stack overflow * Add testing to verify looping doesn't continue
1 parent 5318bc3 commit de2e504

File tree

2 files changed

+43
-3
lines changed

2 files changed

+43
-3
lines changed

integration-test/test-pages/runtime-checks/pages/basic-run.html

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -137,6 +137,45 @@
137137
];
138138
});
139139

140+
test('Prevent src overloading', async () => {
141+
window.scripty2Ran = false;
142+
const scriptElement = document.createElement('script');
143+
scriptElement.id = 'scripty2';
144+
scriptElement.setAttribute('type', 'application/javascript');
145+
scriptElement.src = 'test://url'
146+
147+
let setCounter = 0
148+
// Pretend to be page overloading the src attribute
149+
Object.defineProperty(scriptElement, 'src', {
150+
get: () => 'invalid',
151+
set: () => {
152+
setCounter++
153+
}
154+
})
155+
156+
const getAttribute = scriptElement.getAttribute('src');
157+
// Should increment setCounter
158+
scriptElement.src = 'test://other'
159+
// Should NOT increment setCounter
160+
scriptElement.setAttribute('src', 'bloop');
161+
162+
document.body.appendChild(scriptElement);
163+
const hadInspectorNode = scriptElement === document.querySelector('ddg-runtime-checks:last-of-type');
164+
// Continue to modify the script element after it has been added to the DOM
165+
scriptElement.madeUpProp = 'val';
166+
const instanceofResult = scriptElement instanceof HTMLScriptElement;
167+
const scripty = document.querySelector('#scripty2');
168+
169+
return [
170+
{ name: 'hadInspectorNode', result: hadInspectorNode, expected: true },
171+
{ name: 'expect script to match', result: scripty, expected: scriptElement },
172+
{ name: 'scripty.getAttribute', result: getAttribute, expected: 'test://url' },
173+
{ name: 'setAttribute does not loop', result: setCounter, expected: 1 },
174+
{ name: 'scripty.type', result: scripty.type, expected: 'application/javascript' },
175+
{ name: 'scripty.id', result: scripty.id, expected: 'scripty2' }
176+
];
177+
});
178+
140179
// eslint-disable-next-line no-undef
141180
renderResults();
142181
</script>

src/features/runtime-checks.js

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -253,16 +253,17 @@ class DDGRuntimeChecks extends HTMLElement {
253253
getAttribute (name, value) {
254254
if (shouldFilterKey(this.#tagName, 'attribute', name)) return
255255
if (supportedSinks.includes(name)) {
256-
return this[name]
256+
// Use Reflect to avoid infinite recursion
257+
return Reflect.get(DDGRuntimeChecks.prototype, name, this)
257258
}
258259
return this._callMethod('getAttribute', name, value)
259260
}
260261

261262
setAttribute (name, value) {
262263
if (shouldFilterKey(this.#tagName, 'attribute', name)) return
263264
if (supportedSinks.includes(name)) {
264-
this[name] = value
265-
return
265+
// Use Reflect to avoid infinite recursion
266+
return Reflect.set(DDGRuntimeChecks.prototype, name, value, this)
266267
}
267268
return this._callMethod('setAttribute', name, value)
268269
}

0 commit comments

Comments
 (0)