From 70d165e8bcbed247347f3459a06339c2f65a13fb Mon Sep 17 00:00:00 2001 From: johnnyq Date: Mon, 27 Jul 2026 20:58:37 -0400 Subject: [PATCH] Claim shared item views atomically and log guest audit IPs --- functions/security.php | 21 +++++ guest/guest_download_file.php | 10 ++- guest/guest_post.php | 4 + guest/guest_view_item.php | 24 ++++-- guest/includes/inc_all_guest.php | 4 + itflow-share-view-toctou.patch | 143 +++++++++++++++++++++++++++++++ 6 files changed, 194 insertions(+), 12 deletions(-) create mode 100644 itflow-share-view-toctou.patch diff --git a/functions/security.php b/functions/security.php index cda79fc6..24c9632f 100644 --- a/functions/security.php +++ b/functions/security.php @@ -225,3 +225,24 @@ zgjRYR/zGN5l+az6RB3+0mJRdZdv/y2aRkBlwTxx2gOrPbQAco4a/IOmkE3EbHe7 return false; } + +// Atomically claim one view against a shared item's view limit. +// Returns true only if this request won the view; false means the share is +// inactive, expired, or out of views. The UPDATE is the claim, so concurrent +// requests cannot all pass - call this before any shared content is disclosed. +function claimSharedItemView($item_id) { + global $mysqli; + + $item_id = intval($item_id); + + mysqli_query($mysqli, "UPDATE shared_items + SET item_views = item_views + 1 + WHERE item_id = $item_id + AND item_active = 1 + AND item_expire_at > NOW() + AND (COALESCE(item_view_limit, 0) = 0 OR item_views < item_view_limit)" + ); + + // -1 (query error) and 0 (limit reached / revoked / expired) both deny + return mysqli_affected_rows($mysqli) === 1; +} diff --git a/guest/guest_download_file.php b/guest/guest_download_file.php index 22610f3d..c6cd12b0 100644 --- a/guest/guest_download_file.php +++ b/guest/guest_download_file.php @@ -56,6 +56,12 @@ if (isset($_GET['id']) && isset($_GET['key'])) { exit("Item cannot be viewed at this time (No file, may have been deleted)."); } + // Claim the view before the file is served. The checks above stay as a + // fast path for messaging - this UPDATE is what enforces the limit. + if (!claimSharedItemView($item_id)) { + exit("Item cannot be viewed at this time (view limit exceeded)."); + } + $file_name = escapeSql($file_row['file_name']); $file_reference_name = escapeSql($file_row['file_reference_name']); $client_id = intval($file_row['file_client_id']); @@ -67,10 +73,6 @@ if (isset($_GET['id']) && isset($_GET['key'])) { header('Content-Disposition: attachment; filename=' . $file_name); readfile($file_path); - // Update file view count - $new_item_views = $item_views + 1; - mysqli_query($mysqli, "UPDATE shared_items SET item_views = $new_item_views WHERE item_id = $item_id"); - //Logging logAudit("Share", "View", "Downloaded shared file $file_name via link", $client_id); diff --git a/guest/guest_post.php b/guest/guest_post.php index 7cec8a95..6ab945d7 100644 --- a/guest/guest_post.php +++ b/guest/guest_post.php @@ -13,6 +13,10 @@ session_start(); require_once "../includes/inc_set_timezone.php"; // Must be included after session_start to work +// logAudit() reads these globals - without them guest audit rows have no IP +$session_ip = escapeSql(getIP()); +$session_user_agent = escapeSql($_SERVER['HTTP_USER_AGENT']); + if (isset($_GET['accept_quote'], $_GET['url_key'])) { $quote_id = intval($_GET['accept_quote']); diff --git a/guest/guest_view_item.php b/guest/guest_view_item.php index e6b04178..68f87fc4 100644 --- a/guest/guest_view_item.php +++ b/guest/guest_view_item.php @@ -128,6 +128,14 @@ if ($item_type == "Document") { exit(); } + // Claim the view before any content is disclosed + if (!claimSharedItemView($item_id)) { + echo "
Item cannot be viewed at this time. Check with the person that sent you this link to ensure it is correct and has not expired.
"; + require_once $_SERVER['DOCUMENT_ROOT'] . '/includes/footer.php'; + + exit(); + } + $doc_title = escapeHtml($doc_row['document_name']); $doc_title_escaped = escapeSql($doc_row['document_name']); $doc_content = $purifier->purify($doc_row['document_content']); @@ -135,10 +143,6 @@ if ($item_type == "Document") { echo "

$doc_title

"; echo "
$doc_content
"; - // Update document view count - $new_item_views = $item_views + 1; - mysqli_query($mysqli, "UPDATE shared_items SET item_views = $new_item_views WHERE item_id = $item_id"); - // Logging $name = mysqli_real_escape_string($mysqli, $doc_title); logAudit("Share", "View", "Viewed shared $item_type $doc_title_escaped via link", $client_id); @@ -176,6 +180,14 @@ if ($item_type == "Document") { exit(); } + // Claim the view before the credential is decrypted or rendered + if (!claimSharedItemView($item_id)) { + echo "
Item cannot be viewed at this time. Check with the person that sent you this link to ensure it is correct and has not expired.
"; + require_once $_SERVER['DOCUMENT_ROOT'] . '/includes/footer.php'; + + exit(); + } + $credential_id = intval($credential_row['credential_id']); $credential_name = escapeHtml($credential_row['credential_name']); $credential_uri = escapeHtml($credential_row['credential_uri']); @@ -254,10 +266,6 @@ if ($item_type == "Document") { NOW() ++ AND (COALESCE(item_view_limit, 0) = 0 OR item_views < item_view_limit)" ++ ); ++ ++ // -1 (query error) and 0 (limit reached / revoked / expired) both deny ++ return mysqli_affected_rows($mysqli) === 1; ++} +diff --git a/guest/guest_download_file.php b/guest/guest_download_file.php +index 22610f3..c6cd12b 100644 +--- a/guest/guest_download_file.php ++++ b/guest/guest_download_file.php +@@ -56,6 +56,12 @@ if (isset($_GET['id']) && isset($_GET['key'])) { + exit("Item cannot be viewed at this time (No file, may have been deleted)."); + } + ++ // Claim the view before the file is served. The checks above stay as a ++ // fast path for messaging - this UPDATE is what enforces the limit. ++ if (!claimSharedItemView($item_id)) { ++ exit("Item cannot be viewed at this time (view limit exceeded)."); ++ } ++ + $file_name = escapeSql($file_row['file_name']); + $file_reference_name = escapeSql($file_row['file_reference_name']); + $client_id = intval($file_row['file_client_id']); +@@ -67,10 +73,6 @@ if (isset($_GET['id']) && isset($_GET['key'])) { + header('Content-Disposition: attachment; filename=' . $file_name); + readfile($file_path); + +- // Update file view count +- $new_item_views = $item_views + 1; +- mysqli_query($mysqli, "UPDATE shared_items SET item_views = $new_item_views WHERE item_id = $item_id"); +- + //Logging + logAudit("Share", "View", "Downloaded shared file $file_name via link", $client_id); + +diff --git a/guest/guest_post.php b/guest/guest_post.php +index 7cec8a9..6ab945d 100644 +--- a/guest/guest_post.php ++++ b/guest/guest_post.php +@@ -13,6 +13,10 @@ session_start(); + + require_once "../includes/inc_set_timezone.php"; // Must be included after session_start to work + ++// logAudit() reads these globals - without them guest audit rows have no IP ++$session_ip = escapeSql(getIP()); ++$session_user_agent = escapeSql($_SERVER['HTTP_USER_AGENT']); ++ + if (isset($_GET['accept_quote'], $_GET['url_key'])) { + + $quote_id = intval($_GET['accept_quote']); +diff --git a/guest/guest_view_item.php b/guest/guest_view_item.php +index e6b0417..68f87fc 100644 +--- a/guest/guest_view_item.php ++++ b/guest/guest_view_item.php +@@ -128,6 +128,14 @@ if ($item_type == "Document") { + exit(); + } + ++ // Claim the view before any content is disclosed ++ if (!claimSharedItemView($item_id)) { ++ echo "
Item cannot be viewed at this time. Check with the person that sent you this link to ensure it is correct and has not expired.
"; ++ require_once $_SERVER['DOCUMENT_ROOT'] . '/includes/footer.php'; ++ ++ exit(); ++ } ++ + $doc_title = escapeHtml($doc_row['document_name']); + $doc_title_escaped = escapeSql($doc_row['document_name']); + $doc_content = $purifier->purify($doc_row['document_content']); +@@ -135,10 +143,6 @@ if ($item_type == "Document") { + echo "

$doc_title

"; + echo "
$doc_content
"; + +- // Update document view count +- $new_item_views = $item_views + 1; +- mysqli_query($mysqli, "UPDATE shared_items SET item_views = $new_item_views WHERE item_id = $item_id"); +- + // Logging + $name = mysqli_real_escape_string($mysqli, $doc_title); + logAudit("Share", "View", "Viewed shared $item_type $doc_title_escaped via link", $client_id); +@@ -176,6 +180,14 @@ if ($item_type == "Document") { + exit(); + } + ++ // Claim the view before the credential is decrypted or rendered ++ if (!claimSharedItemView($item_id)) { ++ echo "
Item cannot be viewed at this time. Check with the person that sent you this link to ensure it is correct and has not expired.
"; ++ require_once $_SERVER['DOCUMENT_ROOT'] . '/includes/footer.php'; ++ ++ exit(); ++ } ++ + $credential_id = intval($credential_row['credential_id']); + $credential_name = escapeHtml($credential_row['credential_name']); + $credential_uri = escapeHtml($credential_row['credential_uri']); +@@ -254,10 +266,6 @@ if ($item_type == "Document") { + +