From e11d8f32527d4415c7a3d3f790a4b121c3076ea6 Mon Sep 17 00:00:00 2001 From: johnnyq Date: Thu, 16 Jul 2026 19:18:08 -0400 Subject: [PATCH] Harden checkFileUpload: drop content hashing for random storage names Replace md5(file_contents)+randomString(2) naming with randomString(32). No longer reads the file into memory (removes file_get_contents), so validation is O(1) regardless of size or upload count. Add is_uploaded_file() and UPLOAD_ERR_OK checks, use pathinfo() for extension extraction, and return false consistently on all failures (oversize previously returned a truthy error string that callers treated as a valid filename). --- functions/sanitize.php | 56 ++++++++++++++++++++---------------------- 1 file changed, 27 insertions(+), 29 deletions(-) diff --git a/functions/sanitize.php b/functions/sanitize.php index b22edaac..ffe54e01 100644 --- a/functions/sanitize.php +++ b/functions/sanitize.php @@ -106,43 +106,41 @@ function sanitizeFilename($filename, $strict = false) { return $filename; } -// Pass $_FILE['file'] to check an uploaded file before saving it -function checkFileUpload($file, $allowed_extensions) -{ - // Variables - $name = $file['name']; - $tmp = $file['tmp_name']; - $size = $file['size']; - - $extarr = explode('.', $name); - $extension = strtolower(end($extarr)); - - // Check a file is actually attached/uploaded - if ($tmp === '') { - // No file uploaded +// Validate a single $_FILES[...] entry before saving it. +// Returns a safe, unguessable storage filename (random + original extension) +// on success, or false on ANY failure. The client's own filename is never used +// on disk, so path tricks and double extensions (evil.php.jpg) are irrelevant. +function checkFileUpload($file, $allowed_extensions) { + // Must be a well-formed single-file upload (reject arrays / malformed entries) + if (!isset($file['tmp_name'], $file['error'], $file['size'], $file['name']) + || is_array($file['tmp_name'])) { return false; } - // Check the extension is allowed - if (!in_array($extension, $allowed_extensions)) { - // Extension not allowed + // Must be a successful upload + if ($file['error'] !== UPLOAD_ERR_OK) { return false; } - // Check the size is under 500 MB - $maxSizeBytes = 500 * 1024 * 1024; // 500 MB - if ($size > $maxSizeBytes) { - return "File size exceeds the limit."; + // Must be a genuine HTTP upload, not an arbitrary server path + if (!is_uploaded_file($file['tmp_name'])) { + return false; } - // Read the file content - $fileContent = file_get_contents($tmp); + // Reject empty files and enforce the 500 MB ceiling + $size = (int) $file['size']; + if ($size <= 0 || $size > 500 * 1024 * 1024) { + return false; + } - // Hash the file content using SHA-256 - $hashedContent = hash('md5', $fileContent); + // Allow-list check against the FINAL extension only, case-insensitive + $extension = strtolower(pathinfo($file['name'], PATHINFO_EXTENSION)); + if ($extension === '' || !in_array($extension, $allowed_extensions, true)) { + return false; + } - // Generate a secure filename using the hashed content - $secureFilename = $hashedContent . randomString(2) . '.' . $extension; - - return $secureFilename; + // Unguessable storage name. We deliberately do NOT hash file contents: + // randomString(32) already guarantees uniqueness, and hashing would mean + // reading the whole file into memory (up to 500 MB) for no downstream use. + return randomString(32) . '.' . $extension; }