Security vulnerabilities and mitigations around users
Hello folks.
We’ve been both audited AND attacked on moqui instances. I am listing below what I found so far, and I’m also providing PRs for fixes to some of these problems. We might require CVE declarations, or not depending on how you wish to handle this.
Brute force user info extraction
A successful attack was made for gathering user information. The attack is a combination of:
brute force login end-point, the end-point differentiates between non existent users and those with incorrect password.
find email addresses of identified existing users by calling resetPassword from Login.xml
Now although we can tarpit the resetPassword end-point as it calls a service, we cannot tarpit the login end-points because they are not services and upon testing it seems tarpits only apply on services, not screens or transitions.
XSS in all render modes
go to any text field in vuet, qvt or regular apps render mode and write something like the following
{{constructor.constructor('alert(1)')()}}
you will notice that the alert window will popup. We need to sanitize text fields in all render modes.
PRs to fix some of these vulnerabilities
First, create a unified login service in UserServices, framework PR here and in the same PR also hide email address from resetPassword end-point
Rewire login end-points in runtime to use above service runtime PR here
I have more vulnerabilities to address later on, but these seem to be the most critical for now. I look forward to hearing your thoughts on them.
To protect login from being abused, I just come up with a simple method:
Write a filter for jetty server which will read a txt file containing a list of ip address allowed to have access to login page under webroot. This text file will be reloaded automatically if its content being changed.
This is just a hack. Not a well thought method.
PopRestStore, I did not do anything with it. Because it is open for the public. Meaning that login rest service is exposed to public as it is.
So if something is open to the public (rest.xml or PopRestStore) does not mean you don’t secure it against brute-force attacks. You must at a minimum throttle down the frequency or “tarpit” the attack, otherwise people can run scripts against your server all day to try and expose either usernames or weak passwords.
So my patches above, convert both Login.xml and rest.xml to utilize a new service that I created in UserServices.xml that is a centralized login service. The benefit of this design is that first, we fix all login problems in one place, second, we can now tarpit this service and thus limit the rate of attack.
PopRestStore should also utilize this service and only add the additional logic related to e-commerce. This way everything depends on a single service that is well maintained and does everything for consumers. Note that only login is not a service, everything else is actually a service like resetPassword, updatePassword and so on. So my focus was on fixing login.
Next, I also changed the error message of resetPassword not to expose the email that the reset password is being sent to. This prevents attackers from building a database of users (username and email) by combining the above attacks.
On the XSS issue, or maybe better described as a template injection attack, here is an issue where this came up a few months ago:
There is a commit in moqui-runtime mentioned there, and a few follow up commits from more testing and fixing.
When you found this issue was it a version of moqui-runtime before or after these commits? After those changes I did consider that filtering input is probably worth it, but there are a few issues with that. One of them is the general principles of changing something in one place (input) to fix an issue in another place (output rendering). Part of the problem in this case is if something gets into the database anyway then the input filtering won’t matter. Another part of that problem for this case is not all output for Moqui uses Vue JS for rendering, or other {{}} based string expansion.
BTW, you mentioned regular apps render mode, plain HTML… do you have a test case for that? This sort of template interpolation is only done by Vue JS, which is only used in the vuet and qvt render modes (ie under /vapps and /qapps).
The main question I have is whether you found this issue with a version of moqui-runtime from before or after the changes mentioned above. If you found it with those changes in place then there is still a hole somewhere, so more details would be helpful.
Other may disagree, but FWIW I this is fine to discuss in the open. It is a somewhat minor issue and more importantly has already been discussed in the open and already has a mitigation in place… or a partial mitigation if more holes are found!
On the two part issue of brute force valid username determination and getting an email address from a username, I think it is worth changing the message to not include the email address. That is likely to cause some user support challenges as users don’t always know which email address to check and the email address associated with their account may not be one they expect, but unless someone actually finds that their helpdesk load for forgotten passwords increases because of this then it’s probably safe to guess that it would only cause minor and infrequent issues for users and helpdesk personnel.
On the brute force valid username discovery, I had a chat with Michael about that to think it over (this thread was one topic that came up on our call today). The only solution we role played that is likely to work is client IP based velocity limits.
The current velocity limits in Moqui will not help. The wrong password time delay for incorrect password on a correct account is per user, and so are artifact tarpits (which can be used for screens and transitions, but not for entity ops, IIRC). In other words even existing artifact tarpits would not help with this because they are tarpits for authenticated users, not for pre-auth requests.
The idea of dumbing down the message to not distinguish between invalid username and incorrect password would also not help, that just slightly changes the approach needed for a brute force attack. With an incorrect password temporary account lockout all one has to do is try each candidate username with X different passwords, X being the number of login attempts before account lock which can be determined by manual testing, and if it does lock you out you know the username is valid.
Are there any other approaches that anyone would like to propose? The best one seems to be client IP address based velocity limits, which is a bit of a pain to build and there are ways to partly workaround even that, but even the most determined attacker can only get access to a limited number of client IP addresses (okay, a large group that is determined enough can get millions by routing traffic through botnets of compromised machines, but I don’t think that is so common).
I’ll take a closer look at the PR you submitted and comment on it over there on GitHub.
First of all, you’re right “/apps” is not affected I misspoke it’s only qvt and vuet modes. With the latest version of the framework and runtime components I still get the popup when executing {{constructor.constructor('alert(1)')()}} into any text field.
Regarding the email address you mentioned that it might cause some confusion. In this, how about obfuscating the email, something like a********b@c*****d, that could perhaps strike a balance between user confusion vs security and information collection.
It’s unfortunate indeed that we don’t have tarpits on guest accounts or visits or IP addresses. This means my only option until we implement this is to perhaps secure login, signup, resetPassword to be all behind something like google recaptcha. But this means now that I need to override Login.xml, rest.xml and UserServices.xml to wire in captcha to all of those, and I have to maintain a synched version with the community.
So how about this? let’s override all these entry points to accept an optional recaptcha token, with some kind of setting to the framework, such that if captcha is enabled then it will block all these end-points. I already have recaptcha enforced on signup for one of the systems
Oh and one last thing, I was not able to activate tarpits on screens or transitions. Maybe I’m doing something wrong?
input {{constructor.constructor('alert(1)')()}} into any of the text fields
observe the popup executing
So, latest version of moqui and runtime is definitely still exposed. I will try to check the macros to see how to fix it, but this repeat at least confirms on vanilla moqui system the vulnerability persists.
I was able to reproduce this. The Find Options inputs were being displayed in 2 places in the qvt form-list header, and 1 place in the vuet one. Here are the changes to add v-pre in these 3 places:
Hopefully the scan you did was more thorough that the manual testing that Michael and I did for the first set of changes for this a while back, and that this is the last of the places that need v-pre. Chances are there are more… lots of code in there.