search

LibreHealthIO / lh-ehr

LibreHealth EHR - Free Open Source Electronic Health Records
Other
241 stars 264 forks source link

Create a whitelist to prevent LFI attacks & directory traversals #1608

Closed maggienegm closed 4 years ago

maggienegm commented 4 years ago

Removing the url parameter at the end of the app's main endpoint (libreehr/interface/main/tabs/main.php?url=TAB_ONE_DEFAULT) will prevent users from uploading harmful files aka local file inclusion attacks.

maggienegm commented 4 years ago

By default, two tabs are chosen and displayed when a user logs in. A user can go in the settings and change that if needed. (One way to do this is to choose “User Preferences” under username in top right corner > Appearance tab.) I don't think there's a need for users to be able to "upload" a tab locally using the url parameter.

aethelwulffe commented 4 years ago

Better check Role Based Menus and Single Application Access before you chop that out. 

Don't remove a stone wall from the middle of a field until you make sure it isn't actually a Levee....

On 4/22/2020 4:04 PM, Maggie Negm wrote:

By default, two tabs are chosen and displayed when a user logs in. A user can go in the settings and change that if needed. (One way to do this is to choose “User Preferences” under username in top right corner > Appearance tab.) I don't think there's a need for users to be able to "upload" a tab locally using the url parameter.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/LibreHealthIO/lh-ehr/pull/1608#issuecomment-618010338, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAEHGF5SRLMVFOEQQLKPLCLRN5EWXANCNFSM4MOOLRYQ.

robbyoconnor commented 4 years ago

@aethelwulffe would you like me to add you back to the org so you can review these? We have a lot of work on this likely to last the summer and could use another eye

KoniKodes commented 4 years ago

@aethelwulffe It is good to see you sir. Your guidance would be much appreciated by my team members.

They have also been working on the GACL and ACL roles - hoping this will be helpful as well.

maggienegm commented 4 years ago

Thanks for the guidance @aethelwulffe . I'll take a look, and lesson learned!

maggienegm commented 4 years ago

My new changes start a whitelist of acceptable file names from the interface/main/main_screen.php file, since it uses the url parameter. This will help prevent LFI attacks and directory traversals.

I had trouble finding how Role Based Menus and Single Page Apps use the url parameter as suggested by @aethelwulffe . I would appreciate any guidance regarding this!