Nosial/FederationServer
2
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions 1

Add permission check and improve error handling in UploadAttachment
Some checks are pending
CI / debug (push) Waiting to run
Details
CI / check-phpunit (push) Waiting to run
Details
CI / check-phpdoc (push) Waiting to run
Details
CI / release (push) Waiting to run
Details
CI / generate-phpdoc (push) Blocked by required conditions
Details
CI / test (push) Blocked by required conditions
Details
CI / release-documentation (push) Blocked by required conditions
Details
CI / release-artifacts (push) Blocked by required conditions
Details

Browse source
This commit is contained in:
netkas 2025-06-06 01:04:21 -04:00
parent 4d83948374
commit 04b4714da6
Signed by: netkas
GPG key ID: 4D8629441B76E4CC
1 changed files with 14 additions and 27 deletions
Download patch file Download diff file Expand all files Collapse all files

41
src/FederationServer/Methods/Attachments/UploadAttachment.php
View file

@ -20,20 +20,19 @@
{ {
/** /**
* @inheritDoc * @inheritDoc
* @throws RequestException
*/ */
public static function handleRequest(): void public static function handleRequest(): void
{ {
$evidenceUuid = FederationServer::getParameter('evidence'); $operator = FederationServer::requireAuthenticatedOperator();
if($evidenceUuid === null) if(!$operator->canManageBlacklist())
{ {
throw new RequestException('Evidence UUID is required', 400); throw new RequestException('Insufficient Permissions to upload attachments', 403);
} }
// Validate evidence UUID exists $evidenceUuid = FederationServer::getParameter('evidence');
if(!Validate::uuid($evidenceUuid) || !EvidenceManager::evidenceExists($evidenceUuid)) if($evidenceUuid === null || !Validate::uuid($evidenceUuid))
{ {
throw new RequestException('Invalid Evidence UUID', 400); throw new RequestException('A valid evidence UUID is required', 400);
} }
try try
@ -49,12 +48,6 @@
throw new RequestException('Evidence not found or database error', 404, $e); throw new RequestException('Evidence not found or database error', 404, $e);
} }
$operator = FederationServer::getAuthenticatedOperator();
if(!$operator->canManageBlacklist())
{
throw new RequestException('Insufficient Permissions to upload attachments', 403);
}
// Verify the file upload field exists // Verify the file upload field exists
if(!isset($_FILES['file'])) if(!isset($_FILES['file']))
{ {
@ -63,7 +56,7 @@
// Ensure only a single file is uploaded // Ensure only a single file is uploaded
$file = $_FILES['file']; $file = $_FILES['file'];
if (!is_array($file) || !isset($file['tmp_name']) || empty($file['tmp_name']) || is_array($file['tmp_name'])) if (!is_array($file) || empty($file['tmp_name']) || is_array($file['tmp_name']))
{ {
throw new RequestException('Invalid file upload or multiple files detected', 400); throw new RequestException('Invalid file upload or multiple files detected', 400);
} }
@ -74,10 +67,9 @@
throw new RequestException('Invalid file size'); throw new RequestException('Invalid file size');
} }
$maxUploadSize = Configuration::getServerConfiguration()->getMaxUploadSize(); if ($file['size'] > Configuration::getServerConfiguration()->getMaxUploadSize())
if ($file['size'] > $maxUploadSize)
{ {
throw new RequestException("File exceeds maximum allowed size ({$maxUploadSize} bytes)", 401); throw new RequestException(sprintf("File exceeds maximum allowed size (%d bytes)", Configuration::getServerConfiguration()->getMaxUploadSize()), 401);
} }
// Validate file upload status // Validate file upload status
@ -103,7 +95,7 @@
// Additional check for path traversal attempts // Additional check for path traversal attempts
$realpath = realpath($file['tmp_name']); $realpath = realpath($file['tmp_name']);
if ($realpath === false || strpos($realpath, sys_get_temp_dir()) !== 0) if ($realpath === false || !str_starts_with($realpath, sys_get_temp_dir()))
{ {
throw new RequestException('Path traversal attempt detected'); throw new RequestException('Path traversal attempt detected');
} }
@ -124,13 +116,8 @@
throw new RequestException('Storage directory is not writable', 500); throw new RequestException('Storage directory is not writable', 500);
} }
// Generate a strong random UUID for the file
$uuid = Uuid::v4()->toRfc4122(); $uuid = Uuid::v4()->toRfc4122();
// Prepare destination path (UUID only, no extension as per requirements)
$destinationPath = $storagePath . DIRECTORY_SEPARATOR . $uuid; $destinationPath = $storagePath . DIRECTORY_SEPARATOR . $uuid;
// Use atomic operations where possible
$tempDestination = $storagePath . DIRECTORY_SEPARATOR . uniqid('tmp_', true); $tempDestination = $storagePath . DIRECTORY_SEPARATOR . uniqid('tmp_', true);
if (!move_uploaded_file($file['tmp_name'], $tempDestination)) if (!move_uploaded_file($file['tmp_name'], $tempDestination))
@ -166,15 +153,15 @@
{ {
// If database insertion fails, remove the file to maintain consistency // If database insertion fails, remove the file to maintain consistency
@unlink($destinationPath); @unlink($destinationPath);
Logger::log()->error(sprintf('Failed to record file upload for evidence %s: %s', $evidenceUuid, $e->getMessage()), $e); Logger::log()->error(sprintf('Failed to record file upload for evidence %s: %s', $evidenceUuid, $e->getMessage()), $e);
throw new RequestException('Failed to record file upload: ' . $e->getMessage()); throw new RequestException('Internal Server Error: Unable to create file attachment record', 500, $e);
} }
catch (Throwable $e) { catch (Throwable $e)
{
// Handle any other unexpected errors // Handle any other unexpected errors
@unlink($destinationPath); @unlink($destinationPath);
Logger::log()->error(sprintf('Unexpected error during file upload for evidence %s: %s', $evidenceUuid, $e->getMessage())); Logger::log()->error(sprintf('Unexpected error during file upload for evidence %s: %s', $evidenceUuid, $e->getMessage()));
throw new RequestException('Unexpected error during file upload: ' . $e->getMessage()); throw new RequestException('Internal Server Error', 500, $e);
} }
finally finally
{ {