From 24a1ed1230cbacff7a02b74b30372a3ee37fff53 Mon Sep 17 00:00:00 2001 From: Jonathan Addington Date: Mon, 31 Mar 2025 14:21:24 -0400 Subject: [PATCH] Fix: Code injection vulnerability in chart rendering MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Added proper input validation before evaluating user input as code - Created isValidChartConfig function to detect and block potentially dangerous patterns - Added JSON.parse as the primary method for handling user input - Added strict mode to prevent many JavaScript exploit techniques - Added additional safety checks and improved error messages Closes #22 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude --- lib/charts.js | 158 ++++++++++++++++++++++++++++++++++++++++++++------ 1 file changed, 140 insertions(+), 18 deletions(-) diff --git a/lib/charts.js b/lib/charts.js index 367fc7d..4944eca 100644 --- a/lib/charts.js +++ b/lib/charts.js @@ -7,6 +7,112 @@ const { fixNodeVmObject } = require('./util'); const { logger } = require('../logging'); const { uniqueSvg } = require('./svg'); +/** + * Validates a string to ensure it only contains a safe chart configuration + * @param {string} input The chart configuration string to validate + * @returns {boolean} True if the input is a safe chart configuration + */ +function isValidChartConfig(input) { + // List of dangerous patterns that could indicate code injection + const dangerousPatterns = [ + /\beval\s*\(/i, // eval() + /\bFunction\s*\(/i, // Function constructor + /\bsetTimeout\s*\(/i, // setTimeout + /\bsetInterval\s*\(/i, // setInterval + /\brequire\s*\(/i, // require + /\bimport\s*\(/i, // import() + /\bprocess\b/i, // process object + /\bglobal\b/i, // global object + /\b__dirname\b/i, // __dirname + /\b__filename\b/i, // __filename + /\bconstructor\b.*\bprototype\b/i, // prototype pollution + /\bObject\s*\.\s*([gs]et)?[pP]rototype[oO]f\b/i, // prototype manipulation + /\bdocument\b/i, // DOM access + /\bwindow\b/i, // window object + /\blocation\b/i, // location object + /\bnavigator\b/i, // navigator object + /\bfetch\b/i, // fetch API + /\bXMLHttpRequest\b/i, // XHR + /\bWebSocket\b/i, // WebSocket + /\balert\b/i, // alert + /\bconfirm\b/i, // confirm + /\bprompt\b/i, // prompt + /\blocalStorage\b/i, // localStorage + /\bsessionStorage\b/i, // sessionStorage + /\bindexedDB\b/i, // indexedDB + /\bfs\b/i, // filesystem + /\bhttps?\b/i, // http/https modules + /\bnet\b/i, // net module + /\bchild_process\b/i, // child_process module + /\bcrypto\b/i, // crypto module + /\bzlib\b/i, // zlib module + /\bdgram\b/i, // dgram module + /<(\w+)>/i, // HTML tags + /\(\s*\)\s*=>/i, // arrow functions + /\bfunction\s*\(/i, // function declarations + /\bnew\s+/i, // new keyword + /\bdelete\b/i, // delete operator + ]; + + // Check if the input contains any dangerous patterns + for (const pattern of dangerousPatterns) { + if (pattern.test(input)) { + return false; + } + } + + // Basic structure validation - ensure the input starts with a chart-like object + const validStartPatterns = [ + /^\s*{/, // Object literal + /^\s*\(\s*{/, // Parenthesized object literal + /^\s*\{\s*type\s*:/i, // Object with type property + ]; + + let isValidStart = false; + for (const pattern of validStartPatterns) { + if (pattern.test(input)) { + isValidStart = true; + break; + } + } + + if (!isValidStart) { + return false; + } + + // Check for balanced curly braces as a basic syntax check + let braceCount = 0; + let inString = false; + let stringChar = ''; + let escaped = false; + + for (let i = 0; i < input.length; i++) { + const char = input[i]; + + if (inString) { + if (escaped) { + escaped = false; + } else if (char === '\\') { + escaped = true; + } else if (char === stringChar) { + inString = false; + } + } else if (char === '"' || char === "'") { + inString = true; + stringChar = char; + } else if (char === '{') { + braceCount++; + } else if (char === '}') { + braceCount--; + if (braceCount < 0) { + return false; // Unbalanced braces + } + } + } + + return braceCount === 0 && !inString; +} + // Polyfills require('canvas-5-polyfill'); global.CanvasGradient = canvas.CanvasGradient; @@ -130,25 +236,41 @@ async function renderChartJs( ) { let chart; if (typeof untrustedChart === 'string') { - // The chart could contain Javascript - run it in a VM. + // First, try to parse as JSON try { - const { getGradientFill, getGradientFillHelper } = getGradientFunctions(width, height); - const chartFunction = new Function( - 'getGradientFill', - 'getGradientFillHelper', - 'pattern', - 'Chart', - `return ${untrustedChart}`, - ); - chart = chartFunction( - getGradientFill, - getGradientFillHelper, - { draw: patternDraw }, - getChartJsForVersion(version), - ); - } catch (err) { - logger.error('Input Error', err, untrustedChart); - return Promise.reject(new Error(`Invalid input\n${err}`)); + chart = JSON.parse(untrustedChart); + } catch (jsonErr) { + // If it's not valid JSON, it might be a JavaScript object literal + // Try to validate and sanitize it before evaluation + if (isValidChartConfig(untrustedChart)) { + try { + const { getGradientFill, getGradientFillHelper } = getGradientFunctions(width, height); + // Use indirect eval in a controlled context + const chartFunction = new Function( + 'getGradientFill', + 'getGradientFillHelper', + 'pattern', + 'Chart', + `"use strict"; return ${untrustedChart}`, + ); + chart = chartFunction( + getGradientFill, + getGradientFillHelper, + { draw: patternDraw }, + getChartJsForVersion(version), + ); + } catch (evalErr) { + logger.error('Input Error', evalErr, untrustedChart); + return Promise.reject(new Error(`Invalid input\n${evalErr}`)); + } + } else { + logger.error('Invalid chart configuration format', jsonErr, untrustedChart); + return Promise.reject( + new Error( + 'Invalid chart configuration. Must be valid JSON or a safe chart object literal.', + ), + ); + } } } else { // The chart is just a simple JSON object.