From b06c2b0726ca1f2c673e1867e14887154eef4f43 Mon Sep 17 00:00:00 2001 From: "google-labs-jules[bot]" <161369871+google-labs-jules[bot]@users.noreply.github.com> Date: Wed, 15 Jul 2026 03:25:25 +0000 Subject: [PATCH] fix(upload_security): eliminate FPs for log messages and internal s3 lookups --- CHANGELOG.md | 3 +++ .../extractors/upload_security.py | 15 ++++++----- tests/test_pentest_regressions.py | 26 +++++++++++++++++++ 3 files changed, 38 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index db53af9..2f870e1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/). ## [Unreleased] +### Changed +- Tightened regexes in `UploadSecurityExtractor` to eliminate false positives for file parsing loops, S3 `getObject` lookups, MIME-type logging, and metadata object key construction. + ### Added - **No-Row-Level-Security detection** (`missing-rls` class, in `schemas.py` + the ledger) — committed diff --git a/src/websec_validator/extractors/upload_security.py b/src/websec_validator/extractors/upload_security.py index dc0c2ba..f1dd9f7 100644 --- a/src/websec_validator/extractors/upload_security.py +++ b/src/websec_validator/extractors/upload_security.py @@ -25,16 +25,19 @@ ALLOW_LIST = re.compile(r"isAllowedMediaType|allowedMimeTypes|allow[_-]?list|whitelist|ALLOWED_(?:MIME|TYPES|EXT)" r"|ACCEPTED_(?:MIME|TYPES?|EXT)|accepted(?:Mime|File|Content)?(?:Types?|Extensions?)" r"|\bfile-type\b|fileTypeFrom|magic[_-]?byte|detectContentType|\.fromBuffer\b|sniff", re.I) -KEY_FROM_NAME = re.compile(r"(?:Key|key|path|filename|filepath|destination|filename\s*\()\s*[:=(][^;\n]{0,90}" - r"\b(?:originalname|originalName|file\.name)\b" - r"|`[^`]*\$\{[^}]*\boriginalname\b[^}]*\}[^`]*`", re.I) -TRUST_CLIENT_MIME = re.compile(r"(?:req\.files?\.[\w$.]*\.|\bfile\.)mimetype\b|headers\[['\"]content-type['\"]\]", re.I) +KEY_FROM_NAME = re.compile(r"(?:Key|key|path|filename|filepath|destination|filename\s*\()\s*[:=(][^;\n,]{0,90}" + r"(?:\b(?:originalname|originalName|file\.name)\b" + r"|`[^`]*\$\{[^}]*\boriginalname\b[^}]*\}[^`]*`)", re.I) +TRUST_CLIENT_MIME = re.compile(r"(?:req\.files?(?:\.[\w$.]*)?\.|\bfile\.)mimetype\s*(?:===?|!==?|\.includes|\.match)" + r"|(?:\bContentType\b|\bmimetype\b|\btype\b)\s*:\s*(?:req\.files?(?:\.[\w$.]*)?\.|\bfile\.)mimetype\b" + r"|if\s*\([^)]*mimetype\b" + r"|headers\[['\"]content-type['\"]\]", re.I) ACCEPT_SVG = re.compile(r"image/svg\+xml|['\"]svg['\"]", re.I) # file-serving: streaming a STORED/PROXIED object back to the client. Tightened to genuine # file-bytes sinks — the old rule matched a bare `getObject` token (a local coercion helper) and a # Prometheus `res.set('Content-Type', registry.contentType)` (the /metrics endpoint), both FPs. -SERVE_FILE = re.compile(r"res\.sendFile|\.sendFile\s*\(|\.getObject\s*\(|createReadStream|proxyMedia" - r"|streamObject|\.pipe\s*\(\s*res\b|fs\.createReadStream", re.I) +SERVE_FILE = re.compile(r"res\.sendFile|\.sendFile\s*\(|proxyMedia" + r"|streamObject|\.pipe\s*\(\s*res\b", re.I) NOSNIFF = re.compile(r"nosniff", re.I) # `Content-Disposition: attachment` fully defeats the MIME-sniff→stored-XSS vector (the browser # downloads instead of rendering), so a serve site that sets it is SAFE even without nosniff. diff --git a/tests/test_pentest_regressions.py b/tests/test_pentest_regressions.py index 734194f..a3c1e28 100644 --- a/tests/test_pentest_regressions.py +++ b/tests/test_pentest_regressions.py @@ -345,6 +345,32 @@ def test_safe_upload_and_serve_clean(self): " res.setHeader('Content-Disposition','attachment'); getObject(key).pipe(res); }\n"}), {}) self.assertEqual(out["findings"], []) + def test_fp_noise_suppressed(self): + # 1. KEY_FROM_NAME FP: Object building, not storage key + ctx1 = repo({"upload.js": "const multer = require('multer'); const upload = multer(); app.post('/upload', upload.single('file'), (req, res) => { const fileMetadata = { key: uuidv4(), size: req.file.size, originalname: req.file.originalname }; await db.save(fileMetadata); });"}) + out1 = UploadSecurityExtractor().extract(ctx1, {}) + self.assertEqual(out1["findings"], []) + + # 2. KEY_FROM_NAME FP: Logging + ctx2 = repo({"upload.js": "const multer = require('multer'); app.post('/upload', multer().single('file'), (req, res) => { logger.info('Upload started', { path: '/var/tmp/upload', file: req.file.originalname }); });"}) + out2 = UploadSecurityExtractor().extract(ctx2, {}) + self.assertEqual(out2["findings"], []) + + # 3. SERVE_FILE FP: internal file processing (not piped to res) + ctx3 = repo({"serve.js": "import { createReadStream } from 'fs'; import csv from 'csv-parser'; const results = []; createReadStream('data.csv').pipe(csv()).on('data', (data) => results.push(data));"}) + out3 = UploadSecurityExtractor().extract(ctx3, {}) + self.assertEqual(out3["findings"], []) + + # 4. SERVE_FILE FP: s3 getObject for config/data reading + ctx4 = repo({"serve.js": "const params = { Bucket: 'my-bucket', Key: 'config.json' }; const data = await s3.getObject(params).promise(); const config = JSON.parse(data.Body.toString());"}) + out4 = UploadSecurityExtractor().extract(ctx4, {}) + self.assertEqual(out4["findings"], []) + + # 5. TRUST_CLIENT_MIME FP: Logging MIME + ctx5 = repo({"upload.js": "const multer = require('multer'); const upload = multer(); app.post('/upload', upload.single('file'), (req, res) => { logger.debug(`Received file ${req.file.originalname} of type ${req.file.mimetype}`); });"}) + out5 = UploadSecurityExtractor().extract(ctx5, {}) + self.assertEqual(out5["findings"], []) + class PiiExposureTests(unittest.TestCase): # N4 MASKER = "export function maskContactPii(c){ return {...c, phone: mask(c.phone)}; }\n"