Fix IDOR: getQrcode/editQrcode/deleteQrcode had no ownership check
Found while auditing the owner-scope SQL fix from the last commit:
getQrcode() queried purely by id with no scope applied at all (not even
the buggy old form), and editQrcode()/deleteQrcode() both call
getQrcode() first but then run their own unscoped where('id', $id) for
the actual update/delete. Net effect: any authenticated admin/user with
edit or delete rights could view, edit, or delete *any other tenant's*
qr code just by guessing/incrementing the id - in the static/dynamic
edit forms, the bulk download/delete endpoint, and the single delete
flow alike.
Fixed by applying qr_apply_owner_scope() in getQrcode() (covers the
edit-prefill and delete-lookup paths, and exits via failure() before
reaching the actual write query if out of scope) and adding it directly
to the update/delete queries in editQrcode()/deleteQrcode() too, for
defense in depth rather than relying solely on the earlier check.
Verified with a two-tenant scenario (separate admin accounts): before
the fix admin B could view/edit-prefill/delete admin A's qr code, after
the fix all three are correctly blocked (404 / "not found" / delete is
silently a no-op) and admin A's code is untouched.
Users.php and presets.php were checked too and already scope correctly
via different, unaffected patterns - this was isolated to the two
Qrcode classes.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Fo3DiRRpmz2DXjD7Uzhc8u
This commit is contained in:
@@ -207,9 +207,12 @@ class Qrcode {
|
|||||||
}
|
}
|
||||||
|
|
||||||
public function getQrcode($id) {
|
public function getQrcode($id) {
|
||||||
|
require_once BASE_PATH . '/includes/security.php';
|
||||||
|
|
||||||
$db = getDbInstance();
|
$db = getDbInstance();
|
||||||
|
|
||||||
$db->where('id', $id);
|
$db->where('id', $id);
|
||||||
|
qr_apply_owner_scope($db);
|
||||||
$result = $db->getOne($this->table);
|
$result = $db->getOne($this->table);
|
||||||
|
|
||||||
if($result !== NULL)
|
if($result !== NULL)
|
||||||
@@ -531,6 +534,7 @@ class Qrcode {
|
|||||||
|
|
||||||
if(!file_exists(SAVED_QRCODE_DIRECTORY.$data_to_db['filename'].'.'.$old_qrcode["format"]) || $data_to_db['filename'] == $input_data["old_filename"]){
|
if(!file_exists(SAVED_QRCODE_DIRECTORY.$data_to_db['filename'].'.'.$old_qrcode["format"]) || $data_to_db['filename'] == $input_data["old_filename"]){
|
||||||
$db->where('id', $input_data["id"]);
|
$db->where('id', $input_data["id"]);
|
||||||
|
qr_apply_owner_scope($db);
|
||||||
$stat = $db->update($this->table, $data_to_db);
|
$stat = $db->update($this->table, $data_to_db);
|
||||||
|
|
||||||
try{
|
try{
|
||||||
@@ -563,6 +567,7 @@ class Qrcode {
|
|||||||
$qrcode = $this->getQrcode($id);
|
$qrcode = $this->getQrcode($id);
|
||||||
|
|
||||||
$db->where('id', $id);
|
$db->where('id', $id);
|
||||||
|
qr_apply_owner_scope($db);
|
||||||
$status = $db->delete($this->table);
|
$status = $db->delete($this->table);
|
||||||
|
|
||||||
if ($status) {
|
if ($status) {
|
||||||
|
|||||||
@@ -192,9 +192,12 @@ class Qrcode {
|
|||||||
}
|
}
|
||||||
|
|
||||||
public function getQrcode($id) {
|
public function getQrcode($id) {
|
||||||
|
require_once BASE_PATH . '/includes/security.php';
|
||||||
|
|
||||||
$db = getDbInstance();
|
$db = getDbInstance();
|
||||||
|
|
||||||
$db->where('id', $id);
|
$db->where('id', $id);
|
||||||
|
qr_apply_owner_scope($db);
|
||||||
$result = $db->getOne($this->table);
|
$result = $db->getOne($this->table);
|
||||||
|
|
||||||
if($result !== NULL)
|
if($result !== NULL)
|
||||||
@@ -376,6 +379,7 @@ class Qrcode {
|
|||||||
|
|
||||||
if(!file_exists(SAVED_QRCODE_DIRECTORY.$data_to_db['filename'].'.'.$old_qrcode["format"]) || $data_to_db['filename'] == $input_data["old_filename"]){
|
if(!file_exists(SAVED_QRCODE_DIRECTORY.$data_to_db['filename'].'.'.$old_qrcode["format"]) || $data_to_db['filename'] == $input_data["old_filename"]){
|
||||||
$db->where('id', $input_data["id"]);
|
$db->where('id', $input_data["id"]);
|
||||||
|
qr_apply_owner_scope($db);
|
||||||
$stat = $db->update($this->table, $data_to_db);
|
$stat = $db->update($this->table, $data_to_db);
|
||||||
|
|
||||||
try{
|
try{
|
||||||
@@ -408,6 +412,7 @@ class Qrcode {
|
|||||||
$qrcode = $this->getQrcode($id);
|
$qrcode = $this->getQrcode($id);
|
||||||
|
|
||||||
$db->where('id', $id);
|
$db->where('id', $id);
|
||||||
|
qr_apply_owner_scope($db);
|
||||||
$status = $db->delete($this->table);
|
$status = $db->delete($this->table);
|
||||||
|
|
||||||
if ($status) {
|
if ($status) {
|
||||||
|
|||||||
Reference in New Issue
Block a user