Security concern: uploadMany with Mjoin returning all records on filtered table
Security concern: uploadMany with Mjoin returning all records on filtered table
I am successfully using uploadMany using Mjoin and reference files table. In some circumstances I am further filtering results returned to client, for example using GET parameter for record id. If that returns non-zero main records, everything is fine, in returned data you have only files associated with main records. But if filtering results in zero main records, you get all existing files on server.
Example
Using modified code from this https://editor.datatables.net/examples/advanced/upload-many.html example:
Editor::inst( $db, 'users' )
->fields(
Field::inst( 'users.id' ),
Field::inst( 'users.first_name' ),
Field::inst( 'users.last_name' ),
)
->join(
Mjoin::inst( 'files' )
->link( 'users.id', 'users_files.user_id' )
->link( 'files.id', 'users_files.file_id' )
->fields(
Field::inst( 'id' )
->upload( Upload::inst( $_SERVER['DOCUMENT_ROOT'].'/uploads/__ID__.__EXTN__' )
->db( 'files', 'id', array(
'filename' => Upload::DB_FILE_NAME,
'filesize' => Upload::DB_FILE_SIZE,
'web_path' => Upload::DB_WEB_PATH,
'system_path' => Upload::DB_SYSTEM_PATH
) )
->validator( Validate::fileSize( 500000, 'Files must be smaller that 500K' ) )
->validator( Validate::fileExtensions( array( 'png', 'jpg', 'jpeg', 'gif' ), "Please upload an image" ) )
)
)
)
->where( 'id', $_GET['userId'] )
->process( $_POST )
->json();
If there is no user with id requested in $_GET['userId'], you get zero records in data array, but all existing server files in files array.
I guess this is not a bug, but it's how Mjoin works. Any ideas how to filter correctly?
This might be huge security concern.
Replies
For such a case I would absolutely suggest you check that the GET parameter is submitted before using it - e.g.:
Depending on the setup, you might want to check if the user has access to the userId value submitted as well. If it is per user filtering that is happening (which it looks like here), then a session variable would normally be preferred over a query parameter.
Allan
Well, that was very simplified example, just a proof-of-concept. We are filtering on much more complicated criteria, every one coming from authenticated, session-based user variables and permissions. Sometimes that results in zero returned rows.
Imagine simple eshop orders table, each order with multiple associated files. Authenticated user can list his/hers own orders, where user id comes from session. Every request and call to API is sanitized and authenticated. User that don't have any order can see all associated files of all other users!
As a workaround we probably can check and manipulate resulting data before calling
json()method, and cleanfilesarray ifdataarray is empty. Even when using more uploadMany fields in single editor (as discussed here ), this seems to be a simple safeguard, though not really solution to primary problem.Good stuff - I expected it was a simplified case, but just needed to make sure
.
Yeah - that's not good. Is that because whatever session variable you are using is not set, and therefore the filter isn't being applied? Can you show me that code? A check for that value being set would be the way to address this I think.
Allan
My real code is really complex, filtering records on multiple criteria. Point is that even when all variables are set correctly, there are situations when no record satisfies criteria, resulting in empty
dataarray.Like my example: eshop user with no orders yet. Or maybe - server side search for non-existent string.
Looking at front-end everything work as expected, datatable showing no records. But if user look into browser dev tools to ajax request, he/she can see all files of other users in
filesarray. These are not used on front-end (table is empty), but user can find them in network communication.