Task 10: Harden file upload validation
Enhanced validateFileUpload() function in functions.php with comprehensive security: - Hardcoded MIME type whitelist per file type (profile_picture, proof_of_payment, document) - Strict file size limits per type (5MB images, 10MB documents) - Extension validation against whitelist - Double extension prevention (e.g., shell.php.jpg) - MIME type verification using finfo - Image validation with getimagesize() - is_uploaded_file() verification - Random filename generation to prevent path traversal Updated file upload handlers: - upload_profile_picture.php - Profile picture uploads (JPEG, PNG, GIF, WEBP, 5MB max) - submit_pop.php - Proof of payment uploads (PDF only, 10MB max) + CSRF validation + audit logging - add_campsite.php - Campsite thumbnail uploads + input validation + CSRF validation + audit logging Security improvements: - All uploads use random filenames to prevent directory traversal - All uploads use secure file permissions (0644) - File validation occurs before move_uploaded_file() - Comprehensive error logging for failed uploads - Audit logging for successful file operations
This commit is contained in:
142
submit_pop.php
142
submit_pop.php
@@ -1,7 +1,6 @@
|
||||
<?php include_once('header02.php');
|
||||
require_once("functions.php");
|
||||
checkUserSession();
|
||||
umask(002); // At the top of the PHP script, before move_uploaded_file()
|
||||
|
||||
|
||||
$user_id = $_SESSION['user_id'] ?? null;
|
||||
|
||||
@@ -11,91 +10,92 @@ if (!$user_id) {
|
||||
|
||||
// Handle POST submission
|
||||
if ($_SERVER['REQUEST_METHOD'] === 'POST') {
|
||||
// CSRF Token Validation
|
||||
if (!isset($_POST['csrf_token']) || !validateCSRFToken($_POST['csrf_token'])) {
|
||||
http_response_code(403);
|
||||
die('Security token validation failed. Please try again.');
|
||||
}
|
||||
|
||||
$eft_id = $_POST['eft_id'] ?? null;
|
||||
$file_name = str_replace(' ', '_', $eft_id);
|
||||
|
||||
|
||||
|
||||
if (!$eft_id || !isset($_FILES['pop_file'])) {
|
||||
echo "<div class='alert alert-danger'>Invalid submission: missing eft_id or file.</div>";
|
||||
echo "<pre>";
|
||||
echo "POST data: " . print_r($_POST, true);
|
||||
echo "FILES data: " . print_r($_FILES, true);
|
||||
echo "</pre>";
|
||||
} else {
|
||||
$file = $_FILES['pop_file'];
|
||||
$target_dir = "uploads/pop/";
|
||||
$target_file = $target_dir . $file_name . ".pdf";
|
||||
|
||||
// Check for upload errors first
|
||||
if ($file['error'] !== UPLOAD_ERR_OK) {
|
||||
echo "<div class='alert alert-danger'>Upload error code: " . $file['error'] . "</div>";
|
||||
// You can decode error code if needed:
|
||||
// https://www.php.net/manual/en/features.file-upload.errors.php
|
||||
exit;
|
||||
}
|
||||
|
||||
// Check for PDF extension
|
||||
$file_type = strtolower(pathinfo($file['name'], PATHINFO_EXTENSION));
|
||||
if ($file_type !== "pdf") {
|
||||
echo "<div class='alert alert-danger'>Only PDF files allowed. You tried uploading: .$file_type</div>";
|
||||
exit;
|
||||
}
|
||||
|
||||
// Make sure target directory exists and writable
|
||||
if (!is_dir($target_dir)) {
|
||||
echo "<div class='alert alert-danger'>Upload directory does not exist: $target_dir</div>";
|
||||
exit;
|
||||
}
|
||||
if (!is_writable($target_dir)) {
|
||||
echo "<div class='alert alert-danger'>Upload directory is not writable: $target_dir</div>";
|
||||
exit;
|
||||
}
|
||||
|
||||
if (move_uploaded_file($file['tmp_name'], $target_file)) {
|
||||
chmod($target_file, 0664);
|
||||
// Update EFT and booking status
|
||||
$payment_type = $_POST['payment_type'] ?? 'booking';
|
||||
|
||||
if ($payment_type === 'membership') {
|
||||
// Update EFT and booking status
|
||||
$stmt1 = $conn->prepare("UPDATE efts SET status = 'PROCESSING' WHERE eft_id = ?");
|
||||
$stmt1->bind_param("s", $eft_id);
|
||||
$stmt1->execute();
|
||||
// Update membership fee status
|
||||
$stmt = $conn->prepare("UPDATE membership_fees SET payment_status = 'PROCESSING' WHERE payment_id = ?");
|
||||
$stmt->bind_param("s", $eft_id);
|
||||
$stmt->execute();
|
||||
} else {
|
||||
// Update EFT and booking status
|
||||
$stmt1 = $conn->prepare("UPDATE efts SET status = 'PROCESSING' WHERE eft_id = ?");
|
||||
$stmt1->bind_param("s", $eft_id);
|
||||
$stmt1->execute();
|
||||
|
||||
$stmt2 = $conn->prepare("UPDATE bookings SET status = 'PROCESSING' WHERE eft_id = ?");
|
||||
$stmt2->bind_param("s", $eft_id);
|
||||
$stmt2->execute();
|
||||
}
|
||||
|
||||
// Send notification email using sendPOP()
|
||||
$fullname = getFullName($user_id); // Assuming this returns "First Last"
|
||||
exit;
|
||||
}
|
||||
|
||||
// Validate file using hardened validation function
|
||||
$validationResult = validateFileUpload($_FILES['pop_file'], 'proof_of_payment');
|
||||
|
||||
if ($validationResult === false) {
|
||||
echo "<div class='alert alert-danger'>Invalid file. Only PDF files under 10MB are allowed.</div>";
|
||||
exit;
|
||||
}
|
||||
|
||||
$target_dir = "uploads/pop/";
|
||||
$randomFilename = $validationResult['filename'];
|
||||
$target_file = $target_dir . $randomFilename;
|
||||
|
||||
// Make sure target directory exists and writable
|
||||
if (!is_dir($target_dir)) {
|
||||
mkdir($target_dir, 0755, true);
|
||||
}
|
||||
|
||||
if (!is_writable($target_dir)) {
|
||||
echo "<div class='alert alert-danger'>Upload directory is not writable: $target_dir</div>";
|
||||
exit;
|
||||
}
|
||||
|
||||
if (move_uploaded_file($_FILES['pop_file']['tmp_name'], $target_file)) {
|
||||
chmod($target_file, 0644);
|
||||
|
||||
// Update EFT and booking status
|
||||
$payment_type = $_POST['payment_type'] ?? 'booking';
|
||||
|
||||
if ($payment_type === 'membership') {
|
||||
// Update EFT and booking status
|
||||
$stmt1 = $conn->prepare("UPDATE efts SET status = 'PROCESSING' WHERE eft_id = ?");
|
||||
$stmt1->bind_param("s", $eft_id);
|
||||
$stmt1->execute();
|
||||
$stmt1->close();
|
||||
|
||||
// Update membership fee status
|
||||
$stmt = $conn->prepare("UPDATE membership_fees SET payment_status = 'PROCESSING' WHERE payment_id = ?");
|
||||
$stmt->bind_param("s", $eft_id);
|
||||
$stmt->execute();
|
||||
$stmt->close();
|
||||
} else {
|
||||
// Update EFT and booking status
|
||||
$stmt1 = $conn->prepare("UPDATE efts SET status = 'PROCESSING' WHERE eft_id = ?");
|
||||
$stmt1->bind_param("s", $eft_id);
|
||||
$stmt1->execute();
|
||||
$stmt1->close();
|
||||
|
||||
$stmt2 = $conn->prepare("UPDATE bookings SET status = 'PROCESSING' WHERE eft_id = ?");
|
||||
$stmt2->bind_param("s", $eft_id);
|
||||
$stmt2->execute();
|
||||
$stmt2->close();
|
||||
}
|
||||
|
||||
// Send notification email using sendPOP()
|
||||
$fullname = getFullName($user_id);
|
||||
$eftDetails = getEFTDetails($eft_id);
|
||||
$modified = str_replace(' ', '_', $eft_id);
|
||||
|
||||
if ($eftDetails) {
|
||||
$amount = "R" . number_format($eftDetails['amount'], 2);
|
||||
$description = $eftDetails['description'];
|
||||
} else {
|
||||
$amount = "R0.00";
|
||||
$description = "Payment"; // fallback
|
||||
$description = "Payment";
|
||||
}
|
||||
|
||||
|
||||
if (sendPOP($fullname, $modified, $amount, $description)) {
|
||||
if (sendPOP($fullname, $randomFilename, $amount, $description)) {
|
||||
$_SESSION['message'] = "Thank you! Your payment proof has been uploaded and notification sent.";
|
||||
} else {
|
||||
$_SESSION['message'] = "Payment uploaded, but notification email could not be sent.";
|
||||
}
|
||||
|
||||
// Log the action
|
||||
auditLog($user_id, 'POP_UPLOAD', 'efts', $eft_id, ['filename' => $randomFilename, 'payment_type' => $payment_type]);
|
||||
|
||||
header("Location: bookings.php");
|
||||
exit;
|
||||
@@ -158,7 +158,7 @@ if (!empty($bannerImages)) {
|
||||
<?php if (count($items) > 0) {?>
|
||||
|
||||
<form enctype="multipart/form-data" method="POST">
|
||||
|
||||
<input type="hidden" name="csrf_token" value="<?php echo generateCSRFToken(); ?>">
|
||||
<div class="row mt-35">
|
||||
<ul class="tickets clearfix">
|
||||
<li>
|
||||
|
||||
Reference in New Issue
Block a user