🔒 Fix Dynamic Code Execution vulnerability in adventure puzzles#27
Conversation
Removes dynamic execution of JavaScript scripts passed from the server to the client using `new Function()`. Replaces dynamic script strings with statically defined `executeSetup` and `executeCleanup` logic mappings for each level ID to close the execution vector. Co-authored-by: lemononmars <26631189+lemononmars@users.noreply.github.com>
|
👋 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. |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
1 participant
🎯 What: The vulnerability fixed is a Dynamic Code Execution vulnerability (effectively an RCE / XSS vector in the browser context) found in
⚠️ Risk: Although the scripts were hardcoded in a backend file (
src/routes/puzzles/adventure/[id].svelte. The application used to accept executablescriptandcleanupstrings from a backend response directly intonew Function(data.script)();.src/lib/data/adventure_data.ts), this architectural pattern is inherently dangerous. If the data source was ever compromised, or if user inputs were later passed into these fields, it would allow a malicious actor to execute arbitrary JavaScript in the user's browser, potentially stealing session data or performing unwanted actions.🛡️ Solution: The unsafe dynamic evaluation
new Function()logic was removed. Thescriptandcleanuppayload fields were completely removed from the data structure (adventure_data.ts) and API responses. The side effects (such as adding meta tags, updatingdocument.title, logging to console) were centralized into explicitly defined and safe static functions (executeSetupandexecuteCleanup) mapping to hardcoded level switch-cases. A minor bug in the original code involving component reuse during SvelteKit routing was also patched to correctly trigger cleanups.PR created automatically by Jules for task 720675484460092573 started by @lemononmars