From 16f0702c0cc62493fbf1f0ecdb76a0175a96bf22 Mon Sep 17 00:00:00 2001 From: Chuck Date: Sun, 11 Jan 2026 14:22:31 -0500 Subject: [PATCH] fix: Harden upload flow - HTTP status check, path normalization, property assignment Fix three security and reliability issues in upload flow: 1. Check HTTP status before calling response.json(): - Prevents JSON parsing errors on non-2xx responses - Properly handles error responses with status codes - Returns error text if available for better debugging - Prevents masking of HTTP errors 2. Normalize uploadedFile.path before using in img src: - Remove leading slashes with replace(/^\/+/, '') - Add single leading slash for image src - Prevents //host/odd paths that could cause security issues - Ensures consistent path format 3. Replace string-based handlers with property assignment: - Replace setAttribute('onchange', ...) with addEventListener('change', ...) - Replace setAttribute('onclick', ...) with addEventListener('click', ...) - Refactor addCustomFeedRow to use DOM manipulation instead of innerHTML - Prevents injection vulnerabilities from string interpolation - Uses property assignment (img.src, input.name, input.value) instead of setAttribute where appropriate These changes improve security by eliminating XSS injection surfaces and improve reliability by properly handling HTTP errors and path formats. --- web_interface/templates/v3/base.html | 183 ++++++++++++++++++--------- 1 file changed, 124 insertions(+), 59 deletions(-) diff --git a/web_interface/templates/v3/base.html b/web_interface/templates/v3/base.html index b427011b..9e42766c 100644 --- a/web_interface/templates/v3/base.html +++ b/web_interface/templates/v3/base.html @@ -4836,53 +4836,100 @@ const newRow = document.createElement('tr'); newRow.className = 'custom-feed-row'; newRow.setAttribute('data-index', newIndex); - newRow.innerHTML = ` - - - - - - - -
- - - No logo -
- - - - - - - - `; + + // Create name cell + const nameCell = document.createElement('td'); + nameCell.className = 'px-4 py-3 whitespace-nowrap'; + const nameInput = document.createElement('input'); + nameInput.type = 'text'; + nameInput.name = `${fullKey}.${newIndex}.name`; + nameInput.value = ''; + nameInput.className = 'block w-full px-2 py-1 border border-gray-300 rounded text-sm'; + nameInput.placeholder = 'Feed Name'; + nameInput.required = true; + nameCell.appendChild(nameInput); + + // Create URL cell + const urlCell = document.createElement('td'); + urlCell.className = 'px-4 py-3 whitespace-nowrap'; + const urlInput = document.createElement('input'); + urlInput.type = 'url'; + urlInput.name = `${fullKey}.${newIndex}.url`; + urlInput.value = ''; + urlInput.className = 'block w-full px-2 py-1 border border-gray-300 rounded text-sm'; + urlInput.placeholder = 'https://example.com/feed'; + urlInput.required = true; + urlCell.appendChild(urlInput); + + // Create logo cell + const logoCell = document.createElement('td'); + logoCell.className = 'px-4 py-3 whitespace-nowrap'; + const logoContainer = document.createElement('div'); + logoContainer.className = 'flex items-center space-x-2'; + + const fileInput = document.createElement('input'); + fileInput.type = 'file'; + fileInput.id = `${fieldId}_logo_${newIndex}`; + fileInput.accept = 'image/png,image/jpeg,image/bmp,image/gif'; + fileInput.style.display = 'none'; + // Use addEventListener instead of string-based onchange to prevent injection + fileInput.addEventListener('change', function(e) { + handleCustomFeedLogoUpload(e, fieldId, newIndex, pluginId, fullKey); + }); + + const uploadButton = document.createElement('button'); + uploadButton.type = 'button'; + uploadButton.className = 'px-2 py-1 text-xs bg-gray-200 hover:bg-gray-300 rounded'; + // Use addEventListener instead of string-based onclick to prevent injection + uploadButton.addEventListener('click', function() { + document.getElementById(`${fieldId}_logo_${newIndex}`).click(); + }); + const uploadIcon = document.createElement('i'); + uploadIcon.className = 'fas fa-upload mr-1'; + uploadButton.appendChild(uploadIcon); + uploadButton.appendChild(document.createTextNode(' Upload')); + + const noLogoSpan = document.createElement('span'); + noLogoSpan.className = 'text-xs text-gray-400'; + noLogoSpan.textContent = 'No logo'; + + logoContainer.appendChild(fileInput); + logoContainer.appendChild(uploadButton); + logoContainer.appendChild(noLogoSpan); + logoCell.appendChild(logoContainer); + + // Create enabled cell + const enabledCell = document.createElement('td'); + enabledCell.className = 'px-4 py-3 whitespace-nowrap text-center'; + const enabledInput = document.createElement('input'); + enabledInput.type = 'checkbox'; + enabledInput.name = `${fullKey}.${newIndex}.enabled`; + enabledInput.checked = true; + enabledInput.value = 'true'; + enabledInput.className = 'h-4 w-4 text-blue-600'; + enabledCell.appendChild(enabledInput); + + // Create remove cell + const removeCell = document.createElement('td'); + removeCell.className = 'px-4 py-3 whitespace-nowrap text-center'; + const removeButton = document.createElement('button'); + removeButton.type = 'button'; + removeButton.className = 'text-red-600 hover:text-red-800 px-2 py-1'; + // Use addEventListener instead of string-based onclick to prevent injection + removeButton.addEventListener('click', function() { + removeCustomFeedRow(this); + }); + const removeIcon = document.createElement('i'); + removeIcon.className = 'fas fa-trash'; + removeButton.appendChild(removeIcon); + removeCell.appendChild(removeButton); + + // Append all cells to row + newRow.appendChild(nameCell); + newRow.appendChild(urlCell); + newRow.appendChild(logoCell); + newRow.appendChild(enabledCell); + newRow.appendChild(removeCell); tbody.appendChild(newRow); } @@ -4960,7 +5007,15 @@ method: 'POST', body: formData }) - .then(response => response.json()) + .then(response => { + // Check HTTP status before parsing JSON + if (!response.ok) { + return response.text().then(text => { + throw new Error(`Upload failed: ${response.status} ${response.statusText}${text ? ': ' + text : ''}`); + }); + } + return response.json(); + }) .then(data => { if (data.status === 'success' && data.data && data.data.files && data.data.files.length > 0) { const uploadedFile = data.data.files[0]; @@ -4972,6 +5027,10 @@ const pathName = existingPathInput ? existingPathInput.name : `${fullKey}.${index}.logo.path`; const idName = existingIdInput ? existingIdInput.name : `${fullKey}.${index}.logo.id`; + // Normalize path: remove leading slashes, then add single leading slash + const normalizedPath = uploadedFile.path.replace(/^\/+/, ''); + const imageSrc = '/' + normalizedPath; + // Clear logoCell and build DOM safely to prevent XSS logoCell.textContent = ''; // Clear existing content @@ -4985,36 +5044,42 @@ fileInput.id = `${fieldId}_logo_${index}`; fileInput.accept = 'image/png,image/jpeg,image/bmp,image/gif'; fileInput.style.display = 'none'; - fileInput.setAttribute('onchange', `handleCustomFeedLogoUpload(event, '${fieldId}', ${index}, '${pluginId}', '${fullKey}')`); + // Use addEventListener instead of string-based onchange to prevent injection + fileInput.addEventListener('change', function(e) { + handleCustomFeedLogoUpload(e, fieldId, index, pluginId, fullKey); + }); // Create upload button const uploadButton = document.createElement('button'); uploadButton.type = 'button'; uploadButton.className = 'px-2 py-1 text-xs bg-gray-200 hover:bg-gray-300 rounded'; - uploadButton.setAttribute('onclick', `document.getElementById('${fieldId}_logo_${index}').click()`); + // Use addEventListener instead of string-based onclick to prevent injection + uploadButton.addEventListener('click', function() { + document.getElementById(`${fieldId}_logo_${index}`).click(); + }); const uploadIcon = document.createElement('i'); uploadIcon.className = 'fas fa-upload mr-1'; uploadButton.appendChild(uploadIcon); uploadButton.appendChild(document.createTextNode(' Upload')); - // Create img element - set src via setAttribute to prevent XSS + // Create img element - use normalized path, set src via property to prevent XSS const img = document.createElement('img'); - img.setAttribute('src', `/${uploadedFile.path}`); - img.setAttribute('alt', 'Logo'); + img.src = imageSrc; // Use property assignment with normalized path + img.alt = 'Logo'; img.className = 'w-8 h-8 object-cover rounded border'; img.id = `${fieldId}_logo_preview_${index}`; - // Create hidden input for path - set value via setAttribute to prevent XSS + // Create hidden input for path - set value via property to prevent XSS const pathInput = document.createElement('input'); pathInput.type = 'hidden'; - pathInput.setAttribute('name', pathName); - pathInput.setAttribute('value', uploadedFile.path); + pathInput.name = pathName; + pathInput.value = uploadedFile.path; - // Create hidden input for id - set value via setAttribute to prevent XSS + // Create hidden input for id - set value via property to prevent XSS const idInput = document.createElement('input'); idInput.type = 'hidden'; - idInput.setAttribute('name', idName); - idInput.setAttribute('value', String(uploadedFile.id)); // Ensure it's a string + idInput.name = idName; + idInput.value = String(uploadedFile.id); // Ensure it's a string // Append all elements to container container.appendChild(fileInput);