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

XSS vulnerability in ThirtyBees #774

Closed
asteinhauser opened this issue Nov 10, 2018 · 13 comments
Closed

XSS vulnerability in ThirtyBees #774

asteinhauser opened this issue Nov 10, 2018 · 13 comments
Labels

Comments

@asteinhauser
Copy link

Please, see video with the exploitation:
thirtybees.zip

@Dh42
Copy link
Contributor

Dh42 commented Nov 11, 2018

I saw this earlier, I am not sure on how much of a risk this actually is.

@asteinhauser
Copy link
Author

The risk is diminished, because planting the exploit probably requires administrator account, but otherwise it has all the consequences of an XSS attack:
https://www.dionach.com/blog/the-real-impact-of-cross-site-scripting
Moreover, thirtybees uses inconsistent sanitization of the baseURL value - sometimes the value URL is sanitized and sometimes not - see the source code:
thirtybees
so there already are mechanisms how to prevent it, they are just skipped sometimes and that allows the exploitation.

@Dh42
Copy link
Contributor

Dh42 commented Nov 11, 2018

You are correct about that. We were actually talking about removing the bsql from the backend this week since once backend access is gained that whole shop is considered lost.

@asteinhauser
Copy link
Author

Losing an administrator account is usually not as dangerous as the possibility of continuous, concealed and unrestricted execution of malicious JavaScript in all client's and administrator's browsers. Manipulation with data and manipulation with code are generally two different things. But the evaluation of security risks in your application is your choice in the end. I'm not aware of the peculiarities of thirtybees and how you deal with code/data separation. If you don't care about it anyway, than keeping the flaw in there is probably not a big difference.

@Dh42
Copy link
Contributor

Dh42 commented Nov 11, 2018

Let me start by saying I value your contribution and it will make it into the next version, especially since it is a simple change.

But with that being said, once the admin account is lost, You have to assume everything is lost. Since we are a plugin / module type software, like most, once you have an admin account you have unfettered access to the database and file system. While a persistent XSS attack is bad, there are actually much worse attacks. I have seen some that started off in the back office where people "upgrade" a payment module a full version, but have added code to send them all of the credit card numbers while still charging things like normal. Because they manipulated the touch times using a shell script, no telling how long they had been in place.

We do value security at thirty bees, not just for our end users, but also the customer data they collect as well. But you have to be prepared for the reality once someone gets back office access a complete system audit needs to take place.

@asteinhauser
Copy link
Author

OK, that makes sense. Thanks for the explanation - the possibility to add plugins with code makes it clear to me.

@Dh42
Copy link
Contributor

Dh42 commented Nov 11, 2018

Yeah, while anything is that is executed through the system is sanitized, the plugins can have files that are meant to be directly accessed, like say someone compiles a plugin with a shell script file. That will give the uploader of the plugin read / write access on the whole server. There is nothing we can really write in to stop that.

@Traumflug
Copy link
Contributor

Moreover, thirtybees uses inconsistent sanitization of the baseURL value - sometimes the value URL is sanitized and sometimes not - see the source code:

Thought about this immediately. A vulnerability in the installer is harmless, because the installer goes away right after installation, but what about all the password fields elsewhere?

A customer entering a malicious password or user name and a merchant looking up this user is certainly a risk.

@asteinhauser
Copy link
Author

Here I have two others:
thirtybees2.zip
The first one works only for minority of browsers (I exploited it with Epiphany), while the second one works universally.

@asteinhauser
Copy link
Author

Here are five more:
prestashop3.zip

@asteinhauser
Copy link
Author

Here are the last two:
thirtybees4.zip

@Traumflug
Copy link
Contributor

Let me express a big THANK YOU, even if I can't move this into the code right now. Great work!

getdatakick added a commit that referenced this issue Sep 18, 2020
Search configuration page allows for XSS injection attack.

Related to #774
getdatakick added a commit that referenced this issue Sep 18, 2020
physical_uri and virtual_uri are used to construct url for various
assets. We need to ensure they contain valid values only.

Related to #774
getdatakick added a commit that referenced this issue Sep 18, 2020
Search term needs to be escaped

Related to #774
getdatakick added a commit that referenced this issue Sep 18, 2020
Selected date range value needs to be escaped to prevent XSS injection.

Related to #774
getdatakick added a commit that referenced this issue Sep 18, 2020
Escape id_state value before rendering it to form javascript

Related to #774
getdatakick added a commit that referenced this issue Sep 18, 2020
We need to properly handle date filter values that are displayed in
breadcrumbs in order to prevent XSS injection.

Related to #774
getdatakick added a commit that referenced this issue Sep 18, 2020
Language fields emitted as part of form javascript needs to be properly
escaped to prevent potential XSS injection

Related to #774
@getdatakick
Copy link
Contributor

I believe all reported issues were fixed by commits listed above. Closing

eschiendorfer pushed a commit to eschiendorfer/thirtybees that referenced this issue Mar 23, 2022
Search configuration page allows for XSS injection attack.

Related to thirtybees#774
eschiendorfer pushed a commit to eschiendorfer/thirtybees that referenced this issue Mar 23, 2022
physical_uri and virtual_uri are used to construct url for various
assets. We need to ensure they contain valid values only.

Related to thirtybees#774
eschiendorfer pushed a commit to eschiendorfer/thirtybees that referenced this issue Mar 23, 2022
Search term needs to be escaped

Related to thirtybees#774
eschiendorfer pushed a commit to eschiendorfer/thirtybees that referenced this issue Mar 23, 2022
Selected date range value needs to be escaped to prevent XSS injection.

Related to thirtybees#774
eschiendorfer pushed a commit to eschiendorfer/thirtybees that referenced this issue Mar 23, 2022
Escape id_state value before rendering it to form javascript

Related to thirtybees#774
eschiendorfer pushed a commit to eschiendorfer/thirtybees that referenced this issue Mar 23, 2022
We need to properly handle date filter values that are displayed in
breadcrumbs in order to prevent XSS injection.

Related to thirtybees#774
eschiendorfer pushed a commit to eschiendorfer/thirtybees that referenced this issue Mar 23, 2022
Language fields emitted as part of form javascript needs to be properly
escaped to prevent potential XSS injection

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

No branches or pull requests

4 participants