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

CSRF in SimpleInvoices allows an attacker to take over the website #270

Open
tgianko opened this issue Mar 20, 2017 · 6 comments
Open

CSRF in SimpleInvoices allows an attacker to take over the website #270

tgianko opened this issue Mar 20, 2017 · 6 comments

Comments

@tgianko
Copy link

tgianko commented Mar 20, 2017

Hi everyone,

I am copying here a message I sent to Richard Rowley a month ago to disclose a couple of CSRF in SimpleInvoices. I assumed he was one of the developers of this project, but it looks like I was wrong. I didn't hear anything from him so I decided to open an issue here.

=====

I found multiple CSRF vulnerabilities in SimpleInvoices 2013.1beta.8. The version of Simple Invoices that I used was taken from Bitnami. Unfortunately, it looks like they have taken it out from their library.

None of the security relevant state-changing operations are protected by an anti-CSRF token. For example, consider the POST request to create a new administrator:

POST /index.php?module=user&view=add ...
[headers w/ cookies]

user_id=test&enabled=1&password_field=123456&submit=Insert+User&role=1&email=admin2%40example.com&op=insert_user

An attacker can create an admin user with a CSRF and take over the entire website. The other operations that I have tested are (but I wouldn't exclude the presence of others):

  • Enable/disable Paypal: /index.php?id=1&module=preferences&view=save
  • Creation of new customers: /index.php?module=customers&view=add
  • Change tax rate: /index.php?id=1&module=tax_rates&view=save
@apmuthu
Copy link
Contributor

apmuthu commented Mar 21, 2017

Thankyou for your persistence.

Please try out the github master version to see if the said vulnerability still exists.

Checkout the commercial service run by the lead dev Justin Kelly and verify whether that the same is present there as well.

Kindly provide code fix that the community can test out and incorporate herein. There is a constant defined that must be present in each signed in php page before output, along with user authentication already available and optionally enabled.

@johnanastasio17
Copy link

I can't recreate the above with the fearless359 version without being first logged in.

@apmuthu
Copy link
Contributor

apmuthu commented Mar 21, 2017

The fearless359 version is a highly modified fork suitable for Mobile / Responsive displays.

We maintain only the master branch.

@tgianko
Copy link
Author

tgianko commented Mar 21, 2017

@johnanastasio17: a CSRF vulnerability (and the attack) exploits existing authenticated user session. You can find more details here: https://www.owasp.org/index.php/Cross-Site_Request_Forgery_(CSRF)_Prevention_Cheat_Sheet

In a CSRF attack, an attacker can create a page with a hidden form that submits a URL to create an admin. Then, he tricks the admin to visit this link. The web browser of the admin will request the URL as prepared by the attacker and, according to the SOP for cookies, the browser will automatically add all cookies (including the session ones). SimpleInvoice will accept the request as it comes from an authenticated user. However, the authenticated user has never indented to do such a request.

To fix this vulnerability, you should look into the synchronizer token in the above URL which makes it very hard for an attacker to guess all parameters of a request.

@johnanastasio17
Copy link

Thanks for the info tgianko. I only run SI on a local machine, but am always looking to learn :-)

@tgianko
Copy link
Author

tgianko commented May 10, 2017

Hi everyone, is there any plan to release a patched version?

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