Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Some general security advise #184

Closed
LilithWittmann opened this issue May 11, 2024 · 5 comments
Closed

Some general security advise #184

LilithWittmann opened this issue May 11, 2024 · 5 comments

Comments

@LilithWittmann
Copy link

LilithWittmann commented May 11, 2024

Dear horilla team,
I just analyzed your system, which was used by a German public sector company (https://zendis.de/), and recommended they shut down their instance based on that 20-minute review. Based on that, I have some advice for you.

  1. You ship your software in Django Debug mode by default and don't mention turning off debug mode in your installation instructions. Also, your demo instance is running in DEBUG MODE. This is a really bad idea.
  2. Same for the recommendation of running Django with the development server. Which is a really really really bad idea. Here you find many options on how to properly deploy a Django app.
  3. You run your own auth and password recovery. Your password recovery method is a list of UUIDs which you keep in memory. This will not work when your application is properly deployed with multiple workers/in a PaaS setup…. My recommendation would be: don't run your own auth. There is a nice auth implementation in Django that you should use.
  4. You serve your static files via whitenoise - which is generally nice. But you don't protect them at all. So everyone who knows the name of a file can access it. This is especially funny in companies who distribute their payslips via this system - which might be named predictable. Better serve your files in a way so that you can check if the user actually has access to them.
  5. You ship your application with a SQLite database with users preconfigured. Don't do that.
  6. You have a bot account and give it a password. This means this is just a regular user that technically can login - a bad idea. Even if you need bot users, you could just not set passwords for them.
  7. While during that review, I did not have time looking too deep into it, I would recommend you to review your authorization system. There seem to be some places you forgot to check e.g. permissions.

While you seem to build a cool software product-wise, please focus more on security as you work in a sensitive area. Please also note that this is not a professional security review but rather me taking a glimpse into your code.

@horilla-opensource
Copy link
Owner

horilla-opensource commented May 11, 2024

Hi @LilithWittmann ,
Thanks for pointing out to improvising the product.
The team is working on this concerns and will add a patch very soon.
We appreciate the time and efforts that you have to taken for testing Horilla.

With Regards,
Team Horilla

@BhuviTheDataGuy
Copy link

Informative, thanks for the contribution @LilithWittmann :)

@horilla-opensource
Copy link
Owner

HI @LilithWittmann ,
We have acknowledged the issues that you have raised.

  1. For the demo instance we have added the debug as True for our testing purpose and we did not intend to use it for production purpose.
  2. We are updating the production deployment documentation for which we'll mentioning the DEBUG attribute to be set to false.
  3. The password auth method has been changed and revised using the default Django's method.
  4. We are working on the method to have file access for the document owners only in defined cases.
  5. The SQLite3 database which we provide is for the testing purpose and we don't recommend using it for production. That database is solely meant for the testing purpose alone.
  6. The bot account user creation method has been changed and the password is not provided now. Also in the initial case, the users won't be able to access the system using the bot account credentials because the authentication works only when an employee records has been created for the same account, but for the bot account no employee records get created which there by restricts the access to the system.
  7. We really appreciate the efforts that you have taken on giving out points on improving Horilla. Thank you.☺️

With Regards,
Team Horilla

@horilla-opensource
Copy link
Owner

Hi @LilithWittmann ,
The necessary suggestion updates have done and pushed to the main code base.

With Regards,
Team Horilla

@horilla-opensource
Copy link
Owner

Hi @LilithWittmann ,
We are closing this issue due to inactivity. It seems that there hasn't been any recent activity or discussion, and we believe it may have been resolved or is no longer relevant.
If you feel this issue is still important and should be addressed, please feel free to reopen it or create a new issue with updated details.

With Regards,
Team Horilla

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants