mirror of
https://github.com/myronblair/tomsjavajive
synced 2026-07-27 16:52:36 -05:00
security: fix unauthenticated file upload/RCE risk, TLS bypass, XSS, and broken gift-card/review columns
- admin/api/upload-splash.php had NO admin-auth check (included the public customer header, not admin/includes/header.php) and both upload endpoints trusted the client-supplied MIME type and filename extension, so an attacker could name a file "shell.php", spoof Content-Type: image/png, and get PHP written into a web-reachable uploads/ directory. Added AdminAuth check and centralized real-content validation (getimagesize + server-side extension mapping) in a new handleImageUpload() helper used by both admin/upload-image.php and admin/api/upload-splash.php. - Removed CURLOPT_SSL_VERIFYPEER => false from the CyberMail email calls in includes/email.php and includes/functions.php (MITM risk on the API key). Rewrote functions.php's sendEmail() as a thin wrapper around Email::send() so there's one implementation instead of two that could drift (this is what had the second copy of the TLS bypass). - Escaped customer-controlled fields (customer_name, tracking info, reset URL) before interpolating into outbound HTML emails — name is free text from registration/checkout with no length/char restriction, so it was stored-HTML-injectable into every transactional email. - Fixed api/redeem-gift-card.php referencing a nonexistent `balance` column on gift_cards (actual column is current_balance) — gift card redemption was completely broken, always returning "no remaining balance". - Fixed api/submit-review.php inserting into nonexistent `content`/`status` columns on reviews (actual columns are `comment`/`is_approved`) — review submission was crashing on every request. - Hardened .htaccess: block /db/*, /.git/*, and *.sql. Live site currently serves db/schema.sql and the full .git directory (including .git/config, which contains a GitHub PAT with push access) over HTTP — the existing config/includes RedirectMatch rules are also not being enforced live, see report for details; this needs a server-level fix too. - Cleaned up README's leftover install instructions pointing at a deleted create-admin.php with a documented default password. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
@@ -16,6 +16,30 @@ RewriteRule ^ %1 [L,R=301]
|
||||
RedirectMatch 403 /config/.*$
|
||||
RedirectMatch 403 /includes/.*\.php$
|
||||
RedirectMatch 403 /install/.*$
|
||||
RedirectMatch 403 /db/.*$
|
||||
|
||||
# Block .git (full source + commit history, including any credentials ever
|
||||
# committed to it) from ever being served. NOTE: on the live server this
|
||||
# repo's .git directory lives inside the docroot itself (deployed via
|
||||
# `git clone` directly into public_html) and was confirmed reachable over
|
||||
# HTTP (.git/config, .git/logs/HEAD, etc. all returned real content) even
|
||||
# though the RedirectMatch rules above were also confirmed NOT blocking
|
||||
# /config/*.php or /includes/*.php live — meaning .htaccess directives are
|
||||
# not being fully honored by the current web server. This needs a server
|
||||
# config fix (vhost-level deny, or moving .git outside the docroot) in
|
||||
# addition to this file — see review notes.
|
||||
RedirectMatch 403 /\.git/.*$
|
||||
RedirectMatch 403 /\.git$
|
||||
|
||||
<FilesMatch "\.sql$">
|
||||
<IfModule mod_authz_core.c>
|
||||
Require all denied
|
||||
</IfModule>
|
||||
<IfModule !mod_authz_core.c>
|
||||
Order allow,deny
|
||||
Deny from all
|
||||
</IfModule>
|
||||
</FilesMatch>
|
||||
|
||||
# Set default charset
|
||||
AddDefaultCharset UTF-8
|
||||
|
||||
@@ -75,10 +75,10 @@ A complete e-commerce platform built with **PHP 8.4** and **MySQL 8.0** for cPan
|
||||
```
|
||||
|
||||
6. **Create Admin User**
|
||||
- Visit: `https://yoursite.com/create-admin.php`
|
||||
- Or import the default admin from schema.sql:
|
||||
- Email: `admin@tomsjavajive.com`
|
||||
- Password: `admin123!`
|
||||
- `create-admin.php` and the seeded default admin account have been removed —
|
||||
insert your own row into `admin_users` directly (via phpMyAdmin/CLI) with a
|
||||
bcrypt hash from `password_hash($password, PASSWORD_BCRYPT, ['cost' => 12])`.
|
||||
Never leave a well-known default password (e.g. `admin123!`) in place.
|
||||
|
||||
7. **Configure Site URL**
|
||||
- Edit `config/config.php`:
|
||||
|
||||
+12
-20
@@ -1,29 +1,21 @@
|
||||
<?php
|
||||
require_once __DIR__ . '/../includes/header.php';
|
||||
require_once __DIR__ . '/../../includes/auth.php';
|
||||
header('Content-Type: application/json');
|
||||
|
||||
if ($_SERVER['REQUEST_METHOD'] !== 'POST' || empty($_FILES['image'])) {
|
||||
if (!AdminAuth::getUser()) {
|
||||
http_response_code(401);
|
||||
echo json_encode(['error' => 'Unauthorized']);
|
||||
exit;
|
||||
}
|
||||
|
||||
if ($_SERVER['REQUEST_METHOD'] !== 'POST') {
|
||||
echo json_encode(['error' => 'No file received']); exit;
|
||||
}
|
||||
|
||||
$file = $_FILES['image'];
|
||||
$allowed = ['image/jpeg','image/png','image/gif','image/webp'];
|
||||
if (!in_array($file['type'], $allowed)) {
|
||||
echo json_encode(['error' => 'Invalid type. Use JPG, PNG, WebP or GIF.']); exit;
|
||||
}
|
||||
if ($file['size'] > 5 * 1024 * 1024) {
|
||||
echo json_encode(['error' => 'File too large (max 5 MB).']); exit;
|
||||
}
|
||||
$result = handleImageUpload('image', __DIR__ . '/../../uploads/splashes/', 'splash_');
|
||||
|
||||
$dir = __DIR__ . '/../../uploads/splashes/';
|
||||
if (!is_dir($dir)) mkdir($dir, 0755, true);
|
||||
|
||||
$ext = strtolower(pathinfo($file['name'], PATHINFO_EXTENSION));
|
||||
$name = 'splash_' . time() . '_' . bin2hex(random_bytes(4)) . '.' . $ext;
|
||||
$path = $dir . $name;
|
||||
|
||||
if (move_uploaded_file($file['tmp_name'], $path)) {
|
||||
echo json_encode(['success' => true, 'url' => '/uploads/splashes/' . $name]);
|
||||
if (isset($result['success'])) {
|
||||
echo json_encode(['success' => true, 'url' => '/uploads/splashes/' . $result['filename']]);
|
||||
} else {
|
||||
echo json_encode(['error' => 'Could not save file.']);
|
||||
echo json_encode(['error' => $result['error']]);
|
||||
}
|
||||
|
||||
+5
-26
@@ -13,36 +13,15 @@ if (!AdminAuth::getUser()) {
|
||||
exit;
|
||||
}
|
||||
|
||||
if ($_SERVER['REQUEST_METHOD'] !== 'POST' || empty($_FILES['image'])) {
|
||||
if ($_SERVER['REQUEST_METHOD'] !== 'POST') {
|
||||
echo json_encode(['error' => 'No file received']);
|
||||
exit;
|
||||
}
|
||||
|
||||
$file = $_FILES['image'];
|
||||
$allowedTypes = ['image/jpeg', 'image/png', 'image/gif', 'image/webp'];
|
||||
$maxSize = 5 * 1024 * 1024; // 5MB
|
||||
$result = handleImageUpload('image', __DIR__ . '/../uploads/products/', 'product_');
|
||||
|
||||
if (!in_array($file['type'], $allowedTypes)) {
|
||||
echo json_encode(['error' => 'Invalid file type. Use JPG, PNG, WebP, or GIF.']);
|
||||
exit;
|
||||
}
|
||||
|
||||
if ($file['size'] > $maxSize) {
|
||||
echo json_encode(['error' => 'File too large. Maximum 5MB.']);
|
||||
exit;
|
||||
}
|
||||
|
||||
$uploadDir = __DIR__ . '/../uploads/products/';
|
||||
if (!is_dir($uploadDir)) {
|
||||
mkdir($uploadDir, 0755, true);
|
||||
}
|
||||
|
||||
$ext = strtolower(pathinfo($file['name'], PATHINFO_EXTENSION));
|
||||
$filename = 'product_' . time() . '_' . bin2hex(random_bytes(4)) . '.' . $ext;
|
||||
$filepath = $uploadDir . $filename;
|
||||
|
||||
if (move_uploaded_file($file['tmp_name'], $filepath)) {
|
||||
echo json_encode(['success' => true, 'url' => '/uploads/products/' . $filename]);
|
||||
if (isset($result['success'])) {
|
||||
echo json_encode(['success' => true, 'url' => '/uploads/products/' . $result['filename']]);
|
||||
} else {
|
||||
echo json_encode(['error' => 'Failed to save file. Check directory permissions.']);
|
||||
echo json_encode(['error' => $result['error']]);
|
||||
}
|
||||
|
||||
@@ -35,7 +35,7 @@ if (!$giftCard) {
|
||||
jsonResponse(['error' => 'Gift card not found or already used'], 404);
|
||||
}
|
||||
|
||||
if ($giftCard['balance'] <= 0) {
|
||||
if ($giftCard['current_balance'] <= 0) {
|
||||
jsonResponse(['error' => 'This gift card has no remaining balance'], 400);
|
||||
}
|
||||
|
||||
@@ -43,15 +43,15 @@ if ($giftCard['expires_at'] && strtotime($giftCard['expires_at']) < time()) {
|
||||
jsonResponse(['error' => 'This gift card has expired'], 400);
|
||||
}
|
||||
|
||||
$amount = $giftCard['balance'];
|
||||
$amount = $giftCard['current_balance'];
|
||||
|
||||
try {
|
||||
// Start transaction
|
||||
db()->query("START TRANSACTION");
|
||||
|
||||
|
||||
// Update gift card balance to 0
|
||||
db()->query(
|
||||
"UPDATE gift_cards SET balance = 0, is_active = 0, updated_at = NOW() WHERE gift_card_id = :id",
|
||||
"UPDATE gift_cards SET current_balance = 0, is_active = 0, updated_at = NOW() WHERE gift_card_id = :id",
|
||||
['id' => $giftCard['gift_card_id']]
|
||||
);
|
||||
|
||||
|
||||
@@ -55,8 +55,8 @@ db()->insert('reviews', [
|
||||
'customer_email' => $customer['email'],
|
||||
'rating' => $rating,
|
||||
'title' => $title,
|
||||
'content' => $content,
|
||||
'status' => 'pending' // Reviews require admin approval
|
||||
'comment' => $content,
|
||||
'is_approved' => 0 // Reviews require admin approval
|
||||
]);
|
||||
|
||||
jsonResponse([
|
||||
|
||||
+11
-11
@@ -75,7 +75,7 @@ class Email {
|
||||
CURLOPT_HTTPHEADER => ['Authorization: Bearer ' . $this->apiKey, 'Content-Type: application/json'],
|
||||
CURLOPT_RETURNTRANSFER => true,
|
||||
CURLOPT_TIMEOUT => 30,
|
||||
CURLOPT_SSL_VERIFYPEER => false,
|
||||
CURLOPT_SSL_VERIFYPEER => true,
|
||||
]);
|
||||
$response = curl_exec($ch);
|
||||
$httpCode = curl_getinfo($ch, CURLINFO_HTTP_CODE);
|
||||
@@ -115,8 +115,8 @@ class Email {
|
||||
);
|
||||
}
|
||||
$html = $this->getTemplate('order_confirmation', [
|
||||
'order_number' => $order['order_number'],
|
||||
'customer_name' => $order['customer_name'] ?? 'Valued Customer',
|
||||
'order_number' => htmlspecialchars($order['order_number']),
|
||||
'customer_name' => htmlspecialchars($order['customer_name'] ?? 'Valued Customer'),
|
||||
'items_html' => $itemsHtml,
|
||||
'subtotal' => formatCurrency($order['subtotal']),
|
||||
'tax' => formatCurrency($order['tax']),
|
||||
@@ -136,11 +136,11 @@ class Email {
|
||||
|
||||
public function sendShippingNotification(array $order): array {
|
||||
$html = $this->getTemplate('shipping_notification', [
|
||||
'order_number' => $order['order_number'],
|
||||
'customer_name' => $order['customer_name'] ?? 'Valued Customer',
|
||||
'tracking_number' => $order['tracking_number'],
|
||||
'tracking_url' => $order['tracking_url'] ?? '#',
|
||||
'carrier' => $order['shipping_carrier'] ?? 'Our shipping partner'
|
||||
'order_number' => htmlspecialchars($order['order_number']),
|
||||
'customer_name' => htmlspecialchars($order['customer_name'] ?? 'Valued Customer'),
|
||||
'tracking_number' => htmlspecialchars($order['tracking_number']),
|
||||
'tracking_url' => htmlspecialchars($order['tracking_url'] ?? '#'),
|
||||
'carrier' => htmlspecialchars($order['shipping_carrier'] ?? 'Our shipping partner')
|
||||
]);
|
||||
return $this->send(
|
||||
$order['customer_email'],
|
||||
@@ -154,8 +154,8 @@ class Email {
|
||||
public function sendPasswordReset(string $email, string $resetToken, string $name = ''): array {
|
||||
$resetUrl = SITE_URL . '/reset-password.php?token=' . $resetToken;
|
||||
$html = $this->getTemplate('password_reset', [
|
||||
'customer_name' => $name ?: 'there',
|
||||
'reset_url' => $resetUrl,
|
||||
'customer_name' => htmlspecialchars($name ?: 'there'),
|
||||
'reset_url' => htmlspecialchars($resetUrl),
|
||||
'expires' => '1 hour'
|
||||
]);
|
||||
return $this->send($email, "Reset Your Password - Tom's Java Jive", $html, null, ['tags' => ['password-reset']]);
|
||||
@@ -163,7 +163,7 @@ class Email {
|
||||
|
||||
public function sendWelcome(string $email, string $name = ''): array {
|
||||
$html = $this->getTemplate('welcome', [
|
||||
'customer_name' => $name ?: 'Coffee Lover',
|
||||
'customer_name' => htmlspecialchars($name ?: 'Coffee Lover'),
|
||||
'shop_url' => SITE_URL . '/shop.php'
|
||||
]);
|
||||
return $this->send($email, "Welcome to Tom's Java Jive!", $html, null, ['tags' => ['welcome']]);
|
||||
|
||||
+61
-23
@@ -349,31 +349,15 @@ function getCartTotal() {
|
||||
}
|
||||
|
||||
/**
|
||||
* Send email via CyberMail API
|
||||
* Send email via CyberMail API.
|
||||
* Thin wrapper around Email::send() (includes/email.php) so there is a
|
||||
* single implementation of the CyberMail HTTP call, TLS settings, and
|
||||
* error handling instead of two copies that can drift out of sync.
|
||||
*/
|
||||
function sendEmail($to, $subject, $htmlContent, $textContent = '') {
|
||||
$apiKey = getSetting('cybermail_api_key', defined('CYBERMAIL_API_KEY') ? CYBERMAIL_API_KEY : '');
|
||||
$from = getSetting('cybermail_from_email', 'noreply@tomsjavajive.com');
|
||||
$fromName = getSetting('cybermail_from_name', "Tom's Java Jive");
|
||||
if (!$apiKey) {
|
||||
error_log('[TJJ sendEmail] CYBERMAIL_API_KEY not configured');
|
||||
return false;
|
||||
}
|
||||
$payload = ['from' => $from, 'from_name' => $fromName, 'to' => $to, 'subject' => $subject, 'html' => $htmlContent];
|
||||
if ($textContent) $payload['text'] = $textContent;
|
||||
$ch = curl_init('https://platform.cyberpersons.com/email/v1/send');
|
||||
curl_setopt_array($ch, [
|
||||
CURLOPT_RETURNTRANSFER => true, CURLOPT_POST => true,
|
||||
CURLOPT_POSTFIELDS => json_encode($payload),
|
||||
CURLOPT_HTTPHEADER => ['Authorization: Bearer ' . $apiKey, 'Content-Type: application/json'],
|
||||
CURLOPT_TIMEOUT => 20, CURLOPT_SSL_VERIFYPEER => false,
|
||||
]);
|
||||
$response = curl_exec($ch);
|
||||
$httpCode = curl_getinfo($ch, CURLINFO_HTTP_CODE);
|
||||
curl_close($ch);
|
||||
if ($httpCode === 202) return true;
|
||||
error_log('[TJJ sendEmail] CyberMail HTTP ' . $httpCode . ' — ' . $response);
|
||||
return false;
|
||||
require_once __DIR__ . '/email.php';
|
||||
$result = emailService()->send($to, $subject, $htmlContent, $textContent ?: null);
|
||||
return $result['success'] ?? false;
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -382,3 +366,57 @@ function sendEmail($to, $subject, $htmlContent, $textContent = '') {
|
||||
function logActivity($action, $details = [], $userId = null) {
|
||||
// Implement activity logging if needed
|
||||
}
|
||||
|
||||
/**
|
||||
* Validate and save an uploaded image, verifying the ACTUAL file content
|
||||
* (not the client-supplied MIME type or filename extension, both of which
|
||||
* are attacker-controlled) before writing it to disk. Used by every admin
|
||||
* image-upload endpoint so the checks live in exactly one place.
|
||||
*
|
||||
* @param string $fieldName Key in $_FILES to read
|
||||
* @param string $destDir Absolute directory to save into (created if missing)
|
||||
* @param string $prefix Filename prefix, e.g. "product_" or "splash_"
|
||||
* @return array ['success' => bool, 'url' => string, 'path' => string] or ['error' => string]
|
||||
*/
|
||||
function handleImageUpload(string $fieldName, string $destDir, string $prefix = 'img_'): array {
|
||||
if (empty($_FILES[$fieldName]) || $_FILES[$fieldName]['error'] !== UPLOAD_ERR_OK) {
|
||||
return ['error' => 'No file received'];
|
||||
}
|
||||
|
||||
$file = $_FILES[$fieldName];
|
||||
|
||||
if ($file['size'] > MAX_UPLOAD_SIZE) {
|
||||
return ['error' => 'File too large. Maximum ' . (MAX_UPLOAD_SIZE / (1024 * 1024)) . 'MB.'];
|
||||
}
|
||||
|
||||
// Never trust $file['type'] (client-supplied) or the extension in
|
||||
// $file['name'] (attacker-supplied) — inspect the real file bytes.
|
||||
$imageInfo = @getimagesize($file['tmp_name']);
|
||||
if ($imageInfo === false) {
|
||||
return ['error' => 'File is not a valid image.'];
|
||||
}
|
||||
|
||||
$extensionByMime = [
|
||||
'image/jpeg' => 'jpg',
|
||||
'image/png' => 'png',
|
||||
'image/gif' => 'gif',
|
||||
'image/webp' => 'webp',
|
||||
];
|
||||
$mime = $imageInfo['mime'];
|
||||
if (!in_array($mime, ALLOWED_IMAGE_TYPES, true) || !isset($extensionByMime[$mime])) {
|
||||
return ['error' => 'Invalid file type. Use JPG, PNG, WebP, or GIF.'];
|
||||
}
|
||||
|
||||
if (!is_dir($destDir)) {
|
||||
mkdir($destDir, 0755, true);
|
||||
}
|
||||
|
||||
$filename = $prefix . time() . '_' . bin2hex(random_bytes(4)) . '.' . $extensionByMime[$mime];
|
||||
$filepath = rtrim($destDir, '/\\') . '/' . $filename;
|
||||
|
||||
if (!move_uploaded_file($file['tmp_name'], $filepath)) {
|
||||
return ['error' => 'Failed to save file. Check directory permissions.'];
|
||||
}
|
||||
|
||||
return ['success' => true, 'filename' => $filename, 'path' => $filepath];
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user