adam/Piwigo
1
0 Fork 0
mirror of https://github.com/Piwigo/Piwigo.git synced 2026-10-31 06:02:00 +01:00
Code Issues 657 Packages Projects Releases Wiki Activity

Deleted User and Browser with old Cookie => Uncaught #376

New Issue
Closed
opened 2026-10-04 23:55:36 +02:00 by adam · 1 comment
adam commented 2026-10-04 23:55:36 +02:00
Owner
Copy link
Copy source

Here are the steps to reproduce:

  1. Create user in Admin-Panel
  2. Login with that user (Auto-Login disabled, in Incognito-Window), leave window open.
  3. Delete that user in Admin-Panel
  4. Reload Incognito-Window, it shows:
Warning: array_merge(): Argument #1 is not an array in /home/xxx/public_html/include/functions_user.inc.php on line 394

Warning: Invalid argument supplied for foreach() in /home/xxx/public_html/include/functions_user.inc.php on line 396

Notice: Undefined index: id in /home/xxx/public_html/include/functions_user.inc.php on line 422

Notice: Undefined index: status in /home/xxx/public_html/include/functions_user.inc.php on line 422


Warning:  [mysql error 1064] You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near '' at line 3

SELECT cat_id
  FROM piwigo_user_access
  WHERE user_id = 
; in /home/xxx/public_html/include/dblayer/functions_mysqli.inc.php on line 830


Fatal error: Call to a member function fetch_assoc() on boolean in /home/xxx/public_html/include/dblayer/functions_mysqli.inc.php on line 891

I think the there is missing some mechanism to check if the user referred in the cookie still exists and if not, the cookie should be invalidated.

If it should be of any importance, I'm using latest chrome (47.0.2526.73 (64-bit)) on Debian GNU/Linux Testing

Here are the steps to reproduce: 1. Create user in Admin-Panel 2. Login with that user (Auto-Login disabled, in Incognito-Window), leave window open. 3. Delete that user in Admin-Panel 4. Reload Incognito-Window, it shows: ``` Warning: array_merge(): Argument #1 is not an array in /home/xxx/public_html/include/functions_user.inc.php on line 394 Warning: Invalid argument supplied for foreach() in /home/xxx/public_html/include/functions_user.inc.php on line 396 Notice: Undefined index: id in /home/xxx/public_html/include/functions_user.inc.php on line 422 Notice: Undefined index: status in /home/xxx/public_html/include/functions_user.inc.php on line 422 Warning: [mysql error 1064] You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near '' at line 3 SELECT cat_id FROM piwigo_user_access WHERE user_id = ; in /home/xxx/public_html/include/dblayer/functions_mysqli.inc.php on line 830 Fatal error: Call to a member function fetch_assoc() on boolean in /home/xxx/public_html/include/dblayer/functions_mysqli.inc.php on line 891 ``` I think the there is missing some mechanism to check if the user referred in the cookie still exists and if not, the cookie should be invalidated. If it should be of any importance, I'm using latest chrome (47.0.2526.73 (64-bit)) on Debian GNU/Linux Testing <!-- github-sync-id:120717608 -->
adam closed this issue 2026-10-04 23:55:36 +02:00
adam commented 2026-10-04 23:55:37 +02:00
Author
Owner
Copy link
Copy source

I can still reproduce this. For example, with the ShareAlbum plugin, this happens easily with 2.10.1.
Steps:

  1. Share an album.
  2. Load that in a fresh browser window.
  3. Cancel the share (which deletes the user)
  4. Share it again and load the new link in the same browser window as step 2

I fixed it like this:

diff --git a/include/user.inc.php b/include/user.inc.php
index ce2f74c86..e844d1436 100644
--- a/include/user.inc.php
+++ b/include/user.inc.php
@@ -19,6 +19,13 @@ if (isset($_COOKIE[session_name()]))
   elseif (!empty($_SESSION['pwg_uid']))
   {
     $user['id'] = $_SESSION['pwg_uid'];
+    $query = 'SELECT id FROM '.USERS_TABLE.' WHERE '.$conf['user_fields']['id'].' = \''.$user['id'].'\'';
+    $row = pwg_db_fetch_assoc(pwg_query($query));
+    if (empty($row)) {
+      // If the user doesn't exist (anymore) reset
+      logout_user();
+      redirect($_SERVER['REQUEST_URI']);
+    }
   }
 }

It might not be the best place to fix it since it adds an extra sql query to every page load. But getuserdata() called by build_user() just expects $user['id'] to be a valid user and nothing has verified that to be true.

With this fix the old invalid session is deleted and the redirect back to the same request_uri generates a new valid session.

I can still reproduce this. For example, with the ShareAlbum plugin, this happens easily with 2.10.1. Steps: 1. Share an album. 2. Load that in a fresh browser window. 3. Cancel the share (which deletes the user) 4. Share it again and load the new link in the same browser window as step 2 I fixed it like this: ```diff diff --git a/include/user.inc.php b/include/user.inc.php index ce2f74c86..e844d1436 100644 --- a/include/user.inc.php +++ b/include/user.inc.php @@ -19,6 +19,13 @@ if (isset($_COOKIE[session_name()])) elseif (!empty($_SESSION['pwg_uid'])) { $user['id'] = $_SESSION['pwg_uid']; + $query = 'SELECT id FROM '.USERS_TABLE.' WHERE '.$conf['user_fields']['id'].' = \''.$user['id'].'\''; + $row = pwg_db_fetch_assoc(pwg_query($query)); + if (empty($row)) { + // If the user doesn't exist (anymore) reset + logout_user(); + redirect($_SERVER['REQUEST_URI']); + } } } ``` It might not be the best place to fix it since it adds an extra sql query to every page load. But `getuserdata()` called by `build_user()` just expects `$user['id']` to be a valid user and nothing has verified that to be true. With this fix the old invalid session is deleted and the redirect back to the same request_uri generates a new valid session. <!-- github-comment-id:548962638 -->
Sign in to join this conversation.
No Branch/Tag Specified
Labels
No items
No labels
Milestone
No items
No Milestone
Projects
Clear projects
No projects
Assignees
Clear assignees
adam (Adam Wojcieszonek)
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: adam/Piwigo#376