fix(security): neutralize spreadsheet formulas in all CSV/export cell-writers (CSV injection cluster) (#949)
Co-authored-by: Paul Nothaft <paul@MacStudio-von-Paul.local>
This commit is contained in:
@@ -1,4 +1,5 @@
|
||||
const express = require('express');
|
||||
const { neutralizeSpreadsheetFormula } = require('../utils/spreadsheetSafe');
|
||||
const router = express.Router();
|
||||
const { adminAuth } = require('../middleware/auth');
|
||||
const { requirePermission } = require('../middleware/permissions');
|
||||
@@ -450,11 +451,13 @@ function convertToCSV(data) {
|
||||
const value = row[header];
|
||||
if (value === null || value === undefined) return '';
|
||||
if (typeof value === 'boolean') return value ? 'yes' : 'no';
|
||||
if (typeof value === 'string'
|
||||
&& (value.includes(',') || value.includes('"') || value.includes('\n') || value.includes('\r'))) {
|
||||
return `"${value.replace(/"/g, '""')}"`;
|
||||
// Formula-neutralize user-controlled cells (guest_name/comment_text)
|
||||
// before quoting — quoting alone doesn't stop `=cmd()` (GHSA-3cw3).
|
||||
const neutralized = neutralizeSpreadsheetFormula(value);
|
||||
if (neutralized.includes(',') || neutralized.includes('"') || neutralized.includes('\n') || neutralized.includes('\r')) {
|
||||
return `"${neutralized.replace(/"/g, '""')}"`;
|
||||
}
|
||||
return value;
|
||||
return neutralized;
|
||||
}).join(',');
|
||||
});
|
||||
|
||||
|
||||
@@ -39,8 +39,13 @@ function serializeGuest(row) {
|
||||
};
|
||||
}
|
||||
|
||||
const { neutralizeSpreadsheetFormula } = require('../utils/spreadsheetSafe');
|
||||
|
||||
function escapeCsvCell(value) {
|
||||
const str = value == null ? '' : String(value);
|
||||
// Neutralize spreadsheet formulas FIRST (a `=cmd()` guest name executes on
|
||||
// open — RFC-4180 quoting doesn't stop it), then quote-wrap (GHSA-wc99 /
|
||||
// GHSA-f4fp).
|
||||
const str = neutralizeSpreadsheetFormula(value);
|
||||
if (/[,"\n\r]/.test(str)) {
|
||||
return `"${str.replace(/"/g, '""')}"`;
|
||||
}
|
||||
|
||||
@@ -502,6 +502,15 @@ router.get('/export', adminAuth, requirePermission('settings.view'), async (req,
|
||||
/**
|
||||
* Helper function to convert data to CSV
|
||||
*/
|
||||
const { neutralizeSpreadsheetFormula } = require('../utils/spreadsheetSafe');
|
||||
|
||||
function csvCell(value) {
|
||||
// Formula-neutralize, then RFC-4180 quote (the previous join('') did
|
||||
// neither — GHSA-37p4).
|
||||
const s = neutralizeSpreadsheetFormula(value);
|
||||
return /[,"\n\r]/.test(s) ? `"${s.replace(/"/g, '""')}"` : s;
|
||||
}
|
||||
|
||||
function convertToCSV(data) {
|
||||
// Simplified CSV conversion for security logs
|
||||
const headers = ['timestamp', 'event_type', 'client_ip', 'details'];
|
||||
@@ -511,8 +520,8 @@ function convertToCSV(data) {
|
||||
log.client_ip,
|
||||
JSON.stringify(log.details || {})
|
||||
]);
|
||||
|
||||
return [headers.join(','), ...rows.map(row => row.join(','))].join('\n');
|
||||
|
||||
return [headers.join(','), ...rows.map(row => row.map(csvCell).join(','))].join('\n');
|
||||
}
|
||||
|
||||
module.exports = router;
|
||||
@@ -1,4 +1,5 @@
|
||||
const archiver = require('archiver');
|
||||
const { neutralizeSpreadsheetFormula } = require('../utils/spreadsheetSafe');
|
||||
const fs = require('fs');
|
||||
const fsp = require('fs').promises;
|
||||
const path = require('path');
|
||||
@@ -261,12 +262,13 @@ function convertToCSV(data) {
|
||||
|
||||
const csvRows = data.map(row => {
|
||||
return headers.map(header => {
|
||||
const value = row[header];
|
||||
// Escape quotes and wrap in quotes if contains comma
|
||||
if (typeof value === 'string' && (value.includes(',') || value.includes('"'))) {
|
||||
// Formula-neutralize before quoting (guest_name/comment_text are
|
||||
// user-controlled); the old check didn't even escape \n/\r (GHSA-q82f).
|
||||
const value = neutralizeSpreadsheetFormula(row[header]);
|
||||
if (value.includes(',') || value.includes('"') || value.includes('\n') || value.includes('\r')) {
|
||||
return `"${value.replace(/"/g, '""')}"`;
|
||||
}
|
||||
return value || '';
|
||||
return value;
|
||||
}).join(',');
|
||||
});
|
||||
|
||||
|
||||
@@ -6,6 +6,7 @@
|
||||
const archiver = require('archiver');
|
||||
const { PassThrough } = require('stream');
|
||||
const { XmpGenerator } = require('./xmpGenerator');
|
||||
const { neutralizeSpreadsheetFormula } = require('../utils/spreadsheetSafe');
|
||||
const { db } = require('../database/db');
|
||||
const path = require('path');
|
||||
const fs = require('fs').promises;
|
||||
@@ -162,7 +163,10 @@ class PhotoExportService {
|
||||
|
||||
const csvContent = [
|
||||
headers.join(','),
|
||||
...rows.map(row => row.map(cell => `"${String(cell).replace(/"/g, '""')}"`).join(','))
|
||||
// Formula-neutralize each cell before quoting — filenames/categories
|
||||
// are user-controlled, and quoting alone doesn't stop `=cmd()`
|
||||
// execution (GHSA-5364).
|
||||
...rows.map(row => row.map(cell => `"${neutralizeSpreadsheetFormula(cell).replace(/"/g, '""')}"`).join(','))
|
||||
].join('\n');
|
||||
|
||||
return {
|
||||
|
||||
Reference in New Issue
Block a user