Page 1 of 1

This bug may cause major data breach

Posted: 20 Sep 2021, 13:08
by Amelion
We are building our CRM system based on Rukovoditel and our Developer found bug which can cause huge data security breach, especially if somebody is using system as Portal for their clients.

Case is related to all Access Groups and with active option: ‘View only records assign to you’.

In the entity where are existing records there is an option ‘Edit’ in Action tab, and there is hidden address, i.e.: https://xxx/index.php?module=items/form ... age[100]=1

When pasting this link to browser and changing [id] for different value (i.e. 2) you can see record with id 2 and you can change fields values even if record is not assigned to you and you are not an Admin or you don’t have permission to edit this record.

So system is not checking ‘Assiged to’ while displaying these records.

This way anyone, regardless of their Access group and edit permissions can see and edit all records in the system.

Also please take a look on Access Rules in entities as access conditions (i.e. edit only when particular status etc.) are not checked while pasting link to your browser.

Re: This bug may cause major data breach

Posted: 20 Sep 2021, 18:07
by support
File from archive replace to modules\items\
Let me know if it fixed.

Re: This bug may cause major data breach

Posted: 20 Sep 2021, 19:12
by eddydeniro
support wrote: 20 Sep 2021, 18:07 File from archive replace to modules\items\
Let me know if it fixed.
Hi Sergey,

I tested what @Amelion pointed out and your replacement file didn't remove the bug.
I think it happened because the access filter relies only on "path" variable, ignoring the id, so the $current_item_id is 0.
If I moved the id to the path (e.g. path=29-2), now the forbidden access is denied.
I'm testing by involving the id into the filter, the access can be properly sorted out.

Re: This bug may cause major data breach

Posted: 20 Sep 2021, 21:54
by support
I see. Load fix again. Let me know.

Re: This bug may cause major data breach

Posted: 21 Sep 2021, 05:45
by eddydeniro
support wrote: 20 Sep 2021, 21:54 I see. Load fix again. Let me know.
I have tested against updating non-assigned records, the file fix the issue.
But since you filtered specifically for items/form, then I tested other request using id:
  1. items/delete
  2. selected Export*
  3. selected Delete*

* just by creating a fake ajax post

Yes, unassigned user (if he has given the related access) can still delete or export other non-assigned records.
Delete-hacking.png
The comment form (also using id) was only displaying errors, not big deal as in production server, errors & warnings are suppressed.

Re: This bug may cause major data breach

Posted: 21 Sep 2021, 10:23
by support
Found this issue too. Replace files to:

Code: Select all

modules\items\actions\select_items.php
modules\items\module_top.php
Let me know if it works ok now.

Re: This bug may cause major data breach

Posted: 22 Sep 2021, 04:58
by eddydeniro
support wrote: 21 Sep 2021, 10:23 Found this issue too. Replace files to:

Code: Select all

modules\items\actions\select_items.php
modules\items\module_top.php
Let me know if it works ok now.
I have tested the two files and the access privilege is now properly granted. It blocks edit/delete or selected delete/export other records not belong to the assignee.
However for illegal access to items/delete, the dialogue still popped up even though the delete process is blocked afterwards.
Illegal-delete.png
Just crossed my mind, is it better to modify the path variable without losing the id? Considering that "$current_entity_id -$current_item_id " format is actually doing its job properly.
I'm testing by just adding this to the original file and it can block the illegal update and delete directly.

Code: Select all

//this one line below is line 23 in original file.
$current_item_id = (isset($current_item_array[1]) ? (int)$current_item_array[1] : 0);

//this is the fix to block illegal access
if(!$current_item_id && isset($_GET['id'])){
  $current_item_id = _GET('id');
  $current_path = $current_entity_id.'-'.$current_item_id;
}