Bug 567213 - Vulnerability in Mosquitto configuration file parsing
Summary: Vulnerability in Mosquitto configuration file parsing
Status: RESOLVED FIXED
Alias: None
Product: Community
Classification: Eclipse Foundation
Component: Vulnerability Reports (show other bugs)
Version: unspecified   Edit
Hardware: All All
: P3 normal (vote)
Target Milestone: ---   Edit
Assignee: Security vulnerabilitied reported against Eclipse projects CLA
QA Contact:
URL:
Whiteboard:
Keywords: security
Depends on:
Blocks:
 
Reported: 2020-09-21 12:59 EDT by Roland Sako CLA
Modified: 2023-03-31 06:26 EDT (History)
2 users (show)

See Also:


Attachments
Contains the files that trigger the bugs, screenshots, my public key and the binary compiled with ASAN. (1.37 MB, application/x-zip-compressed)
2020-09-21 12:59 EDT, Roland Sako CLA
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Roland Sako CLA 2020-09-21 12:59:57 EDT
Created attachment 284212 [details]
Contains the files that trigger the bugs, screenshots, my public key and the binary compiled with ASAN.

While fuzzing other components, I came across memory corruption issues spotted using AddressSanitizer.

There seem to be heap overflows in both mosquitto_read_file_core and fgets_extending as shown in the screenshots.

Both can be triggered by using mosquitto with the "-c" argument and providing the samples, such as :

mosquitto -c poc_config_read.conf or mosquitto -c poc_fgets_extending.conf

Remarks :
- the target does not crash
- I did not take the time to check for LPE exploitability yet, as it was in-scope of my project. I might dig deeper later on if necessary to write a PoC.

Please, feel free to let me know if you have any other question.

Best regards,
Roland Sako.
Comment 1 Wayne Beaton CLA 2020-09-21 14:37:35 EDT
/cc the project lead.

Our process for resolving vulnerabilities is captured in the handbook.

https://www.eclipse.org/projects/handbook/#vulnerability
Comment 2 Roger Light CLA 2020-09-21 18:39:02 EDT
Thank you for the report Roland. I can confirm that the poc configuration files do make a read to a single byte of memory placed just before a block of memory allocated on the heap.

I am not a security researcher, so forgive me if I have any details wrong and let me know where.

My understanding is that for glibc the first chunk of data prior to a user heap memory allocation (i.e. mem = malloc(1000), then mem[-1]) is a size_t integer which contains the size of the memory allocation for the memory chunk at mem, and some flags.

On a little endian architecture the mem[-1] location is the most significant byte of the size_t. With an initial allocation size of 1000 bytes, we would expect mem[-1] to be 0. This byte would first change when we made a large allocation. On a 64-bit machine, this would be 2^(64-8)+1 = 144PB. On a 32-bit machine, this would be 2^(32-8)+1 = 16MB. The `config__read_file_core` function will read mem[-1] to see whether it is equal to 10 or 13, and if so write a value of 0. That means the allocation would need to be around 1440PB or 160MB on a 64 or 32-bit machine respectively. For a 32-bit machine, after mem[-1] was set to zero then the apparent size would be ~160MB smaller. I'm not sufficiently knowledgeable to say what effect this would have when free(mem) is called later.

On a big endian architecture mem[-1] contains some allocation size information, and the A, M, and P flags. To match the `config__read_file_core` check of 10 we would have to have A=0 (memory is from main heap, which would be the case), M=1 (memory was allocated by a single call to mmap and is not part of the heap - this presumably conflicts with A=0), P=0 (previous chunk is not in use - entirely plausible). The memory size would also have to be the correct size with the lowest byte of the usable size being 0b00001000 - which is never possible when using an allocation multiple of 1000 bytes. I also do not believe this is possible due to the flag conflict, but I may be incorrect.

For the `config__read_file_core` check of 13 we would have to have A=1 (memory is from mmap) M=0 (memory does not come from a single mmap), P=1 (previous chunk is free, entirely plausible) and the memory size limitation applies the same as for a check of 10.

I do not believe either of the big endian mem[-1] == 10 or == 13 could ever be true, so it would never be the case they would be overwritten with a value of 0.

I have not considered the situation for non-glibc allocators.


In conclusion, if an attacker had the capability to create a configuration file that would cause an allocation of at least ~160MB on a 32-bit machine, and after that used this flaw, then they could change the apparent size of the allocation so it was smaller than reality, and potentially cause an issue when the memory was freed later on. It should only be possible to run the broker at the same privilege level as the user currently has. I would consider the situation where a broker initially runs as root using a configuration file that is user editable as a flaw in that particular system, not a flaw in Mosquitto. I am not sufficiently familiar with LPE, but cannot see how privilege escalation could occur. It looks as though an unprivileged user would only be able to attack themselves.

This should definitely be fixed, but at the moment I do not believe it is a serious issue. Please educate me otherwise if necessary.
Comment 3 Wayne Beaton CLA 2021-07-22 10:26:56 EDT
I'm removing the confidentiality flag per policy.

Roger, do you want to keep this issue open, or will you track mitigation elsewhere?
Comment 4 Roger Light CLA 2021-07-22 11:52:39 EDT
This will be fixed in 2.0.12, I honestly don't think there's much need to do anything other than release the fix in this case so the issue can be closed for me.
Comment 5 Wayne Beaton CLA 2021-07-22 12:46:10 EDT
(In reply to Roger Light from comment #4)
> This will be fixed in 2.0.12, I honestly don't think there's much need to do
> anything other than release the fix in this case so the issue can be closed
> for me.

I'll mark it as FIXED. Reopen if you need a CVE.
Comment 6 Roland Sako CLA 2021-07-28 09:41:23 EDT
(In reply to Wayne Beaton from comment #5)
> (In reply to Roger Light from comment #4)
> > This will be fixed in 2.0.12, I honestly don't think there's much need to do
> > anything other than release the fix in this case so the issue can be closed
> > for me.
> 
> I'll mark it as FIXED. Reopen if you need a CVE.

Thanks for the feedback. Would this even get a CVE?
Comment 7 Roger Light CLA 2021-07-28 10:46:07 EDT
Hi Roland,

I'm no expert, but from what I can see I don't understand why it would need one. The scenario as I understand it is if a privileged user edits the configuration file they could trigger an out of bounds access, which could potentially crash the broker or end up with code execution. But the user already has to be privileged in order to edit the config file, so they could have just executed code anyway, or killed the broker.
Comment 8 Roland Sako CLA 2023-03-31 06:26:08 EDT
(In reply to Roger Light from comment #7)
> Hi Roland,
> 
> I'm no expert, but from what I can see I don't understand why it would need
> one. The scenario as I understand it is if a privileged user edits the
> configuration file they could trigger an out of bounds access, which could
> potentially crash the broker or end up with code execution. But the user
> already has to be privileged in order to edit the config file, so they could
> have just executed code anyway, or killed the broker.

Hey Roger,

Cool that it's been fixed. As the -c argument allows specifying the config file's path, it should but does not have to be in a location where privilege access is needed, as stated in mosquitto_conf documentation:

"mosquitto.conf is the configuration file for mosquitto. This file can reside anywhere as long as mosquitto can read it. By default, mosquitto does not need a configuration file and will use the default values listed below"

In that case, there was a potential for local privilege escalation, upon being able to edit this file.

Cheers,
Roland.