password file parser allows adversaries to modify the loaded password file instance and authenticate as another client

[eclipsewebmaster]

From the Eclipse bugzilla:

Hi,

There is a bug in the function used to parse the password file in the MQTT implementation. The function is pwfile__parse(...) and it appears in the file security_default.c .

The bug could allow an adversary to modify the loaded instance of the password file, authenticate as another client or create a DoS.

Adversary's Model:
In order for the adversary to exploit the bug she needs to be able to provide her credentials to the administrator of the broker. This could be done in case there is a service which is used to register new clients.

Bug Details:
The problem lies in the following lines of code

         while(!feof(pwfile)){
	if(fgets(buf, 256, pwfile)){
             ...

used to iterate over the username:passwords pairs in the password file. This implementation assumes that each pair has length maximum 256 bytes which is not a requirement from the MQTT standard.

In case the parser finds a pair with length longer than 256 bytes (e.g. 300), it will read the first 256 bytes, and at the next iteration it will read the rest (e.g. 44). This is the way fgets(...) works.

This allows an adverary to inject in his username (or password) another username:password pair.

Example 1 (without TLS): For instance, in case the broker is not using TLS the following password file:

john:BBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBadmin:mypass
admin:1234

has an injection in the password of the malicious user john (i.e. BBBBB....BBBBadmin:mypass). The parser will load the pairs:

          john:BBBB.....BBBBB
          admin:mypass
          admin:1234

which allows john to authenticate as admin providing mypass as password.

Example 2 (with TLS): A similar problem can occur when the broker has configured with TLS. However, this time the injection needs to occur at the username. For instance the following username

BBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBadmin:$6$Z0CpPwNLEKDcDNKe$7WuqsJTu45uQDXsSWJKJes4iLEij1muwZjPHmJZgtlNcd9t/o74xGrZ5yGxQm4hCNR2fYBBXhA0VOqP2yTMXRw==

has injected the pair

admin:$6$Z0CpPwNLEKDcDNKe$7WuqsJTu45uQDXsSWJKJes4iLEij1muwZjPHmJZgtlNcd9t/o74xGrZ5yGxQm4hCNR2fYBBXhA0VOqP2yTMXRw==

On load, the parser will read the first 256 bytes (B...BBB) and then will load the injected pair, which again permits the adversary to log in as admin.

[karlp]

The wording of this is wildly dramatic, given that you need to be able to edit the password file. Totally agree that validation of the file isn't what it should be, but "adversaries can modify the loaded file" is not even factually correct.

[ghost]

Hi,

I am the one who detected this. It is not dramatic at all and you probably did not understand the issue. The adversary needs to provide only a password and a username NOT to be able to modify the password file.

This could happen if you set up a service for your clients so they can register credentials to your broker. Nothing more is needed.

I think this is a serious issue and deserves a CVE.

[karlp]

yes, if you set up a service that lets people modify the password file! like really? that's a pretty major extra step that you're depending on.

[ghost]

Hi karlp,

No, there is no requirement that the adversary needs to modify the password file or that there is a strange service that you are describing. There is the standard service that asks you to fill out

                      username:...
                      password:...

The adversary can then choose her username and password in such a way that she can authenticate as another user. For instance in the first example that I gave the adversary will give

                      username: john
                      password: BBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBadmin:mypass

and then you will store them in the password file as

john:BBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBadmin:mypass

while for the second example the adversary gives

                       username: BBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBadmin
                       password:mypass

and after you apply the hash you store in the password file

BBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBadmin:$6$Z0CpPwNLEKDcDNKe$7WuqsJTu45uQDXsSWJKJes4iLEij1muwZjPHmJZgtlNcd9t/o74xGrZ5yGxQm4hCNR2fYBBXhA0VOqP2yTMXRw==

Nothing more is needed. The two examples that I have are POC. I tried them and they work. Please read it more carefully and you will see what I mean.

[ralight]

This is now fixed, thanks for the report.

My summary of the problem:

If you:

Let your users create usernames and passwords
Use file based passwords
Write your own tool to manage the password file rather than using mosquitto_passwd
Write your tool so that new entries are always at the beginning of the file, not at the end
Don't do any of your own validation

Then in those circumstances, yes, this it would be possible for a malicious user to produce this problem. That is a very specific set of circumstances though.

[ghost]

Thanks for this.
I have two questions :

1. Could we assign a cve to this ?
2. Where should we post security bugs ? I thought the right place would be bugzila but then they just moved my issue here.

[ralight]

Details on reporting potential security issues are in SECURITY.md: https://github.com/eclipse/mosquitto/blob/master/SECURITY.md

I'm very reluctant to go down the CVE route with this because I don't think I can assign impact etc. values, because it depends on the system that you've built, not on mosquitto. If I had decided to have a system like this with user provided usernames and passwords, the first thing I would have used was mosquitto_passwd, which is the only tool and method provided for maintaining password files. If you use mosquitto_passwd, this problem doesn't exist. If I had decided to make my own implementation, then my implementation would have appended new users to the end of the password file, at which point there is no problem either. Both cases have CVSS score 0.

It seems as though that everything provided by mosquitto works fine together without any problems. It is possible to create the examples you suggest, but they don't have any impact - it is not possible to authenticate as an existing user. Your example has decided to make their own implementation without checking the requirements of the password file format, and hasn't done any validation either. Again, I'm not disputing that this should be fixed, and it has been, but I don't see how this is particularly different to, for example, a bug existing in your tool that is creating the password file where it sets the password for every single user to "password". That would allow anyone to log in as the "admin" user, but it certainly isn't a problem in mosquitto.