search

AlainBenbassat / eu.businessandcode.notepermissions

Other
0 stars 4 forks source link

CiviCRM Export data overrides security #2

Open chumkui opened 2 years ago

chumkui commented 2 years ago

Hi Alain - we have been using the extension for a while now (it's great, thank you) but by chance noticed an issue today when we did an export on staff members.

I think this is also a general Civi issue in term of exporting data, but raising it with you as it particualrly affects additional privacy levels.

If you add notes that are not supposed to be visible to others - either your additional privacy levels or even just 'Author only', you can simply do an Export contacts, choose all primary fields and you get to see the notes.

From my testing, as long as someone has 'View only' on a contact record, they can export to see hidden notes that would be hidden for that person via the UI - not a situation we want. I might just be missing the obvious of course...!

I am going to try and lockdown the export for the moment, I think there is a way to hide it on the menu with an extension until I can be sure we are secure.

AlainBenbassat commented 2 years ago

Thanks @chumkui for reporting this. Maybe by implementing the hook hook_civicrm_export the private notes can be hidden. I am happy to accept your pull request with this change. Otherwise I'll implement it later this month.

chumkui commented 2 years ago

Thanks Alain - you probably would not want me touching code :-)

chumkui commented 2 years ago

OK Alain - you shamed me into looking at this last night :-) As a stop gap I created my very first Civi extension. All it does is remove the notes column from any final export by using an unset on the two arrays that hold the column and header row. Ugly but effective...

I did take a look through the debugger at how the export processor builds up everything to create the table that I can hook into, but I lost the will to live.... I cannot get my head around how I only remove the notes that have a privacy of X, but its a bit beyond me at the moment. I do not develop on a daily basis and so this is challenge for me. Throw me a hint and I'll take a look!

Thanks again for the hook hint, I've at least solved the immediate issue and from an information leakage/privacy point of view, it is quite a big step forward. Many thanks again.

mlutfy commented 1 year ago

I think it's a core bug, not specific to this extension, so I reported it to the security team.

I wrote a slightly different extension that takes inspiration from this one, but my client had very specific requirements, so I wrote this:

chumkui commented 1 year ago

Many thanks mlutty - I'll take a look. They have deprecated hook_civicrm_notePrivacy, so will need to revisit a couple of things I've done in the past.