Security hardening: CSRF, rate limiting, session/password policy, audit log
Fixes critical pre-existing issues found during review: bulk_action.php had no auth check at all (unauthenticated download/delete of any qrcode) and built a table name from unwhitelisted user input (SQL injection); the QR generator classes wrote files from unvalidated filename/format, allowing path traversal and arbitrary file writes. Also pins chillerlan/php-qrcode to 5.0.5 since master now requires PHP 8.4, breaking the PHP 8.3 build. - CSRF tokens on all POST forms and the bulk_action.php JSON endpoint - Login rate limiting (5 attempts / 15 min) via new login_attempts table - Hardened sessions: httponly/samesite cookies, 30 min idle timeout, session regeneration on login - Forced password change for the default superadmin/superadmin account - Server-side validation in Users/DynamicQrcode/Qrcode classes - Audit log table for auth, user, and qrcode actions - Checked-in db schema (db/init.sql, migrations/) instead of relying on an opaque prebuilt db image - Production docker-compose with Nginx + php-fpm instead of the PHP dev server Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
@@ -56,6 +56,8 @@ class DynamicQrcode {
|
||||
* We save into db the url of qrcode image
|
||||
*/
|
||||
public function addQrcode($input_data) {
|
||||
$this->validateLink($input_data['link'] ?? '');
|
||||
|
||||
if($input_data['id_owner'] != "")
|
||||
$data_to_db['id_owner'] = $input_data['id_owner'];
|
||||
else
|
||||
@@ -79,6 +81,8 @@ class DynamicQrcode {
|
||||
*
|
||||
*/
|
||||
public function editQrcode($input_data) {
|
||||
$this->validateLink($input_data['link'] ?? '');
|
||||
|
||||
if($input_data['id_owner'] != "")
|
||||
$data_to_db['id_owner'] = $input_data['id_owner'];
|
||||
else
|
||||
@@ -117,6 +121,17 @@ class DynamicQrcode {
|
||||
}
|
||||
|
||||
|
||||
/**
|
||||
* Server-side validatie van de redirect-link (verplicht, max. 500 tekens per kolomdefinitie).
|
||||
*/
|
||||
private function validateLink($link) {
|
||||
$link = trim((string) $link);
|
||||
|
||||
if ($link === '' || strlen($link) > 500) {
|
||||
$this->failure('Link is required and must be at most 500 characters.');
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Flash message Failure process
|
||||
*/
|
||||
|
||||
@@ -20,6 +20,8 @@ class Qrcode {
|
||||
private string $table;
|
||||
private string $redirect_url;
|
||||
|
||||
const ALLOWED_FORMATS = ['png', 'gif', 'jpeg', 'jpg', 'svg', 'svgbw', 'eps'];
|
||||
|
||||
/**
|
||||
*
|
||||
*/
|
||||
@@ -43,6 +45,33 @@ class Qrcode {
|
||||
{
|
||||
}
|
||||
|
||||
/**
|
||||
* Voorkomt path traversal / arbitrary file write via een gemanipuleerde bestandsnaam.
|
||||
*/
|
||||
private function sanitizeFilename($filename) {
|
||||
$filename = trim((string) $filename);
|
||||
|
||||
if ($filename === '' || strlen($filename) > 45) {
|
||||
$this->failure('Filename must be between 1 and 45 characters.');
|
||||
}
|
||||
|
||||
if (preg_match('#[\\/\\\\]#', $filename) || strpos($filename, '..') !== false || strpos($filename, "\0") !== false) {
|
||||
$this->failure('Filename cannot contain path separators.');
|
||||
}
|
||||
|
||||
return $filename;
|
||||
}
|
||||
|
||||
private function validateFormat($format) {
|
||||
$format = strtolower((string) $format);
|
||||
|
||||
if (!in_array($format, self::ALLOWED_FORMATS, true)) {
|
||||
$this->failure('Invalid qr code format.');
|
||||
}
|
||||
|
||||
return $format;
|
||||
}
|
||||
|
||||
public function getQrcode($id) {
|
||||
$db = getDbInstance();
|
||||
|
||||
@@ -94,6 +123,9 @@ class Qrcode {
|
||||
public function addQrcode($input_data, $data_to_db, $data_to_qrcode) {
|
||||
$options = $this->setOptions($input_data);
|
||||
|
||||
$data_to_db['filename'] = $this->sanitizeFilename($data_to_db['filename']);
|
||||
$data_to_db['format'] = $this->validateFormat($data_to_db['format']);
|
||||
|
||||
$outputInterface = QRGdImagePNG::class;
|
||||
$imageFormat = strtolower($data_to_db['format']);
|
||||
$fileExt = $imageFormat;
|
||||
@@ -305,21 +337,23 @@ class Qrcode {
|
||||
$this->failure('You cannot create a new qr code with an existing name on the server!');
|
||||
|
||||
if ($last_id){
|
||||
audit_log('qrcode_created', $this->table, $last_id);
|
||||
$this->success('Qr code added successfully!');
|
||||
}
|
||||
else {
|
||||
$this->failure('Insert failed: ' . $db->getLastError());
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
/**
|
||||
* Edit qr code
|
||||
*
|
||||
*
|
||||
*/
|
||||
public function editQrcode($input_data, $data_to_db) {
|
||||
$db = getDbInstance();
|
||||
$old_qrcode = $this->getQrcode($input_data["id"]);
|
||||
|
||||
$data_to_db['filename'] = $this->sanitizeFilename($data_to_db['filename']);
|
||||
$data_to_db['qrcode'] = $data_to_db['filename'].'.'.$old_qrcode["format"];
|
||||
|
||||
if(!file_exists(SAVED_QRCODE_DIRECTORY.$data_to_db['filename'].'.'.$old_qrcode["format"]) || $data_to_db['filename'] == $input_data["old_filename"]){
|
||||
@@ -337,6 +371,7 @@ class Qrcode {
|
||||
$this->failure('You cannot edit a qr code with an existing name on the server!');
|
||||
|
||||
if ($stat){
|
||||
audit_log('qrcode_updated', $this->table, $input_data['id']);
|
||||
$this->success('Qr code updated successfully!');
|
||||
}
|
||||
else {
|
||||
@@ -344,10 +379,10 @@ class Qrcode {
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
|
||||
/**
|
||||
* Delete qr code
|
||||
*
|
||||
*
|
||||
*/
|
||||
public function deleteQrcode($id, $async = false) {
|
||||
$db = getDbInstance();
|
||||
@@ -356,7 +391,11 @@ class Qrcode {
|
||||
|
||||
$db->where('id', $id);
|
||||
$status = $db->delete($this->table);
|
||||
|
||||
|
||||
if ($status) {
|
||||
audit_log('qrcode_deleted', $this->table, $id);
|
||||
}
|
||||
|
||||
try{
|
||||
unlink(SAVED_QRCODE_DIRECTORY.$qrcode["filename"].'.'.$qrcode["format"]);
|
||||
}
|
||||
|
||||
@@ -5,6 +5,8 @@ class Qrcode {
|
||||
private string $table;
|
||||
private string $redirect_url;
|
||||
|
||||
const ALLOWED_FORMATS = ['png', 'gif', 'jpeg', 'jpg', 'svg', 'eps'];
|
||||
|
||||
/**
|
||||
*
|
||||
*/
|
||||
@@ -28,6 +30,33 @@ class Qrcode {
|
||||
{
|
||||
}
|
||||
|
||||
/**
|
||||
* Voorkomt path traversal / arbitrary file write via een gemanipuleerde bestandsnaam.
|
||||
*/
|
||||
private function sanitizeFilename($filename) {
|
||||
$filename = trim((string) $filename);
|
||||
|
||||
if ($filename === '' || strlen($filename) > 45) {
|
||||
$this->failure('Filename must be between 1 and 45 characters.');
|
||||
}
|
||||
|
||||
if (preg_match('#[\\/\\\\]#', $filename) || strpos($filename, '..') !== false || strpos($filename, "\0") !== false) {
|
||||
$this->failure('Filename cannot contain path separators.');
|
||||
}
|
||||
|
||||
return $filename;
|
||||
}
|
||||
|
||||
private function validateFormat($format) {
|
||||
$format = strtolower((string) $format);
|
||||
|
||||
if (!in_array($format, self::ALLOWED_FORMATS, true)) {
|
||||
$this->failure('Invalid qr code format.');
|
||||
}
|
||||
|
||||
return $format;
|
||||
}
|
||||
|
||||
public function getQrcode($id) {
|
||||
$db = getDbInstance();
|
||||
|
||||
@@ -79,6 +108,9 @@ class Qrcode {
|
||||
public function addQrcode($input_data, $data_to_db, $data_to_qrcode) {
|
||||
$options = $this->setOptions($input_data);
|
||||
|
||||
$data_to_db['filename'] = $this->sanitizeFilename($data_to_db['filename']);
|
||||
$data_to_db['format'] = $this->validateFormat($data_to_db['format']);
|
||||
|
||||
if(!file_exists(SAVED_QRCODE_DIRECTORY.$data_to_db['filename'].'.'.$data_to_db['format'])){
|
||||
$url =
|
||||
'https://api.qrserver.com/v1/create-qr-code/?data='.
|
||||
@@ -111,6 +143,7 @@ class Qrcode {
|
||||
$this->failure('You cannot create a new qr code with an existing name on the server!');
|
||||
|
||||
if ($last_id){
|
||||
audit_log('qrcode_created', $this->table, $last_id);
|
||||
$this->success('Qr code added successfully!');
|
||||
}
|
||||
else {
|
||||
@@ -126,6 +159,7 @@ class Qrcode {
|
||||
$db = getDbInstance();
|
||||
$old_qrcode = $this->getQrcode($input_data["id"]);
|
||||
|
||||
$data_to_db['filename'] = $this->sanitizeFilename($data_to_db['filename']);
|
||||
$data_to_db['qrcode'] = $data_to_db['filename'].'.'.$old_qrcode["format"];
|
||||
|
||||
if(!file_exists(SAVED_QRCODE_DIRECTORY.$data_to_db['filename'].'.'.$old_qrcode["format"]) || $data_to_db['filename'] == $input_data["old_filename"]){
|
||||
@@ -143,6 +177,7 @@ class Qrcode {
|
||||
$this->failure('You cannot edit a qr code with an existing name on the server!');
|
||||
|
||||
if ($stat){
|
||||
audit_log('qrcode_updated', $this->table, $input_data['id']);
|
||||
$this->success('Qr code updated successfully!');
|
||||
}
|
||||
else {
|
||||
@@ -162,7 +197,11 @@ class Qrcode {
|
||||
|
||||
$db->where('id', $id);
|
||||
$status = $db->delete($this->table);
|
||||
|
||||
|
||||
if ($status) {
|
||||
audit_log('qrcode_deleted', $this->table, $id);
|
||||
}
|
||||
|
||||
try{
|
||||
unlink(SAVED_QRCODE_DIRECTORY.$qrcode["filename"].'.'.$qrcode["format"]);
|
||||
}
|
||||
|
||||
+59
-11
@@ -3,6 +3,8 @@ require_once 'config/config.php';
|
||||
|
||||
class Users
|
||||
{
|
||||
const ALLOWED_TYPES = ['super', 'admin'];
|
||||
|
||||
/**
|
||||
*
|
||||
*/
|
||||
@@ -10,6 +12,25 @@ class Users
|
||||
{
|
||||
}
|
||||
|
||||
/**
|
||||
* Server-side validatie van username/type. Geeft een foutmelding terug (string) of null als geldig.
|
||||
*/
|
||||
private function validateUsernameAndType($username, $type) {
|
||||
if (!is_string($username) || strlen($username) < 3 || strlen($username) > 50) {
|
||||
return 'Username must be between 3 and 50 characters.';
|
||||
}
|
||||
|
||||
if (!preg_match('/^[a-zA-Z0-9._-]+$/', $username)) {
|
||||
return 'Username may only contain letters, numbers, dots, underscores and hyphens.';
|
||||
}
|
||||
|
||||
if (!in_array($type, self::ALLOWED_TYPES, true)) {
|
||||
return 'Invalid user type.';
|
||||
}
|
||||
|
||||
return null;
|
||||
}
|
||||
|
||||
/**
|
||||
*
|
||||
*/
|
||||
@@ -54,6 +75,15 @@ class Users
|
||||
public function addUser($input_data) {
|
||||
$db = getDbInstance();
|
||||
|
||||
$validation_error = $this->validateUsernameAndType($input_data['username'] ?? '', $input_data['type'] ?? '');
|
||||
if ($validation_error !== null) {
|
||||
$this->failure($validation_error, 'Location: user.php');
|
||||
}
|
||||
|
||||
if (!isset($input_data['password']) || strlen($input_data['password']) < 10) {
|
||||
$this->failure('Password must be at least 10 characters long.', 'Location: user.php');
|
||||
}
|
||||
|
||||
$data_to_db["username"] = $input_data["username"];
|
||||
$data_to_db['password'] = password_hash($input_data['password'], PASSWORD_DEFAULT);
|
||||
$data_to_db["type"] = $input_data["type"];
|
||||
@@ -66,8 +96,10 @@ class Users
|
||||
|
||||
$last_id = $db->insert('users', $data_to_db);
|
||||
|
||||
if ($last_id)
|
||||
if ($last_id) {
|
||||
audit_log('user_created', 'user', $last_id);
|
||||
$this->success('User added successfully');
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -77,28 +109,43 @@ class Users
|
||||
public function editUser($input_data) {
|
||||
$db = getDbInstance();
|
||||
|
||||
$query_string = http_build_query(array(
|
||||
'id' => $input_data["id"],
|
||||
'edit' => "true",
|
||||
));
|
||||
|
||||
$validation_error = $this->validateUsernameAndType($input_data['username'] ?? '', $input_data['type'] ?? '');
|
||||
if ($validation_error !== null) {
|
||||
$this->failure($validation_error, 'Location: user.php?'.$query_string);
|
||||
}
|
||||
|
||||
if (isset($input_data['password']) && strlen($input_data['password']) > 0 && strlen($input_data['password']) < 10) {
|
||||
$this->failure('Password must be at least 10 characters long.', 'Location: user.php?'.$query_string);
|
||||
}
|
||||
|
||||
$db->where('username', $input_data['username']);
|
||||
$db->where('id', $input_data["id"], '!=');
|
||||
$row = $db->getOne('users');
|
||||
|
||||
if (!empty($row['username'])) {
|
||||
$query_string = http_build_query(array(
|
||||
'id' => $input_data["id"],
|
||||
'edit' => "true",
|
||||
));
|
||||
$this->failure('Username already exists', 'Location: user.php?'.$query_string);
|
||||
}
|
||||
|
||||
$data_to_db["username"] = $input_data["username"];
|
||||
$data_to_db['password'] = password_hash($input_data['password'], PASSWORD_DEFAULT);
|
||||
$data_to_db["type"] = $input_data["type"];
|
||||
|
||||
// Alleen wachtwoord overschrijven als er een nieuwe waarde is opgegeven.
|
||||
if (!empty($input_data['password'])) {
|
||||
$data_to_db['password'] = password_hash($input_data['password'], PASSWORD_DEFAULT);
|
||||
}
|
||||
|
||||
$db->where('id', $input_data["id"]);
|
||||
$stat = $db->update('users', $data_to_db);
|
||||
|
||||
if ($stat)
|
||||
|
||||
if ($stat) {
|
||||
audit_log('user_updated', 'user', $input_data['id']);
|
||||
$this->success('User updated successfully!');
|
||||
else
|
||||
} else
|
||||
$this->failure('Failed to update User: ' . $db->getLastError());
|
||||
}
|
||||
|
||||
@@ -116,9 +163,10 @@ class Users
|
||||
$db->where('id', $id);
|
||||
$stat = $db->delete('users');
|
||||
|
||||
if ($stat)
|
||||
if ($stat) {
|
||||
audit_log('user_deleted', 'user', $id);
|
||||
$this->info('User deleted successfully!');
|
||||
else
|
||||
} else
|
||||
$this->failure('Unable to delete user');
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user