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.
This commit is contained in:
Chuck
2026-01-11 14:22:31 -05:00
parent 550ab42f9a
commit 16f0702c0c

View File

@@ -4836,53 +4836,100 @@
const newRow = document.createElement('tr'); const newRow = document.createElement('tr');
newRow.className = 'custom-feed-row'; newRow.className = 'custom-feed-row';
newRow.setAttribute('data-index', newIndex); newRow.setAttribute('data-index', newIndex);
newRow.innerHTML = `
<td class="px-4 py-3 whitespace-nowrap"> // Create name cell
<input type="text" const nameCell = document.createElement('td');
name="${fullKey}.${newIndex}.name" nameCell.className = 'px-4 py-3 whitespace-nowrap';
value="" const nameInput = document.createElement('input');
class="block w-full px-2 py-1 border border-gray-300 rounded text-sm" nameInput.type = 'text';
placeholder="Feed Name" nameInput.name = `${fullKey}.${newIndex}.name`;
required> nameInput.value = '';
</td> nameInput.className = 'block w-full px-2 py-1 border border-gray-300 rounded text-sm';
<td class="px-4 py-3 whitespace-nowrap"> nameInput.placeholder = 'Feed Name';
<input type="url" nameInput.required = true;
name="${fullKey}.${newIndex}.url" nameCell.appendChild(nameInput);
value=""
class="block w-full px-2 py-1 border border-gray-300 rounded text-sm" // Create URL cell
placeholder="https://example.com/feed" const urlCell = document.createElement('td');
required> urlCell.className = 'px-4 py-3 whitespace-nowrap';
</td> const urlInput = document.createElement('input');
<td class="px-4 py-3 whitespace-nowrap"> urlInput.type = 'url';
<div class="flex items-center space-x-2"> urlInput.name = `${fullKey}.${newIndex}.url`;
<input type="file" urlInput.value = '';
id="${fieldId}_logo_${newIndex}" urlInput.className = 'block w-full px-2 py-1 border border-gray-300 rounded text-sm';
accept="image/png,image/jpeg,image/bmp,image/gif" urlInput.placeholder = 'https://example.com/feed';
style="display: none;" urlInput.required = true;
onchange="handleCustomFeedLogoUpload(event, '${fieldId}', ${newIndex}, '${pluginId}', '${fullKey}')"> urlCell.appendChild(urlInput);
<button type="button"
onclick="document.getElementById('${fieldId}_logo_${newIndex}').click()" // Create logo cell
class="px-2 py-1 text-xs bg-gray-200 hover:bg-gray-300 rounded"> const logoCell = document.createElement('td');
<i class="fas fa-upload mr-1"></i> Upload logoCell.className = 'px-4 py-3 whitespace-nowrap';
</button> const logoContainer = document.createElement('div');
<span class="text-xs text-gray-400">No logo</span> logoContainer.className = 'flex items-center space-x-2';
</div>
</td> const fileInput = document.createElement('input');
<td class="px-4 py-3 whitespace-nowrap text-center"> fileInput.type = 'file';
<input type="checkbox" fileInput.id = `${fieldId}_logo_${newIndex}`;
name="${fullKey}.${newIndex}.enabled" fileInput.accept = 'image/png,image/jpeg,image/bmp,image/gif';
checked fileInput.style.display = 'none';
value="true" // Use addEventListener instead of string-based onchange to prevent injection
class="h-4 w-4 text-blue-600"> fileInput.addEventListener('change', function(e) {
</td> handleCustomFeedLogoUpload(e, fieldId, newIndex, pluginId, fullKey);
<td class="px-4 py-3 whitespace-nowrap text-center"> });
<button type="button"
onclick="removeCustomFeedRow(this)" const uploadButton = document.createElement('button');
class="text-red-600 hover:text-red-800 px-2 py-1"> uploadButton.type = 'button';
<i class="fas fa-trash"></i> uploadButton.className = 'px-2 py-1 text-xs bg-gray-200 hover:bg-gray-300 rounded';
</button> // Use addEventListener instead of string-based onclick to prevent injection
</td> 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); tbody.appendChild(newRow);
} }
@@ -4960,7 +5007,15 @@
method: 'POST', method: 'POST',
body: formData 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 => { .then(data => {
if (data.status === 'success' && data.data && data.data.files && data.data.files.length > 0) { if (data.status === 'success' && data.data && data.data.files && data.data.files.length > 0) {
const uploadedFile = data.data.files[0]; const uploadedFile = data.data.files[0];
@@ -4972,6 +5027,10 @@
const pathName = existingPathInput ? existingPathInput.name : `${fullKey}.${index}.logo.path`; const pathName = existingPathInput ? existingPathInput.name : `${fullKey}.${index}.logo.path`;
const idName = existingIdInput ? existingIdInput.name : `${fullKey}.${index}.logo.id`; 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 // Clear logoCell and build DOM safely to prevent XSS
logoCell.textContent = ''; // Clear existing content logoCell.textContent = ''; // Clear existing content
@@ -4985,36 +5044,42 @@
fileInput.id = `${fieldId}_logo_${index}`; fileInput.id = `${fieldId}_logo_${index}`;
fileInput.accept = 'image/png,image/jpeg,image/bmp,image/gif'; fileInput.accept = 'image/png,image/jpeg,image/bmp,image/gif';
fileInput.style.display = 'none'; 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 // Create upload button
const uploadButton = document.createElement('button'); const uploadButton = document.createElement('button');
uploadButton.type = 'button'; uploadButton.type = 'button';
uploadButton.className = 'px-2 py-1 text-xs bg-gray-200 hover:bg-gray-300 rounded'; 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'); const uploadIcon = document.createElement('i');
uploadIcon.className = 'fas fa-upload mr-1'; uploadIcon.className = 'fas fa-upload mr-1';
uploadButton.appendChild(uploadIcon); uploadButton.appendChild(uploadIcon);
uploadButton.appendChild(document.createTextNode(' Upload')); 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'); const img = document.createElement('img');
img.setAttribute('src', `/${uploadedFile.path}`); img.src = imageSrc; // Use property assignment with normalized path
img.setAttribute('alt', 'Logo'); img.alt = 'Logo';
img.className = 'w-8 h-8 object-cover rounded border'; img.className = 'w-8 h-8 object-cover rounded border';
img.id = `${fieldId}_logo_preview_${index}`; 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'); const pathInput = document.createElement('input');
pathInput.type = 'hidden'; pathInput.type = 'hidden';
pathInput.setAttribute('name', pathName); pathInput.name = pathName;
pathInput.setAttribute('value', uploadedFile.path); 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'); const idInput = document.createElement('input');
idInput.type = 'hidden'; idInput.type = 'hidden';
idInput.setAttribute('name', idName); idInput.name = idName;
idInput.setAttribute('value', String(uploadedFile.id)); // Ensure it's a string idInput.value = String(uploadedFile.id); // Ensure it's a string
// Append all elements to container // Append all elements to container
container.appendChild(fileInput); container.appendChild(fileInput);