Memory corruption in RAR decoder

[Daniel Axtens]

Hi,

There are some crashes and memory corruption issues in
libarchive's RAR decoder. Most notably, I have observed some
double-frees and heap use-after-frees, both reading and writing. These
have not been detected by previous fuzzing runs because of the CRC
checks in the RAR parser.

The memory corruption seems to arise in ppmd7 decoding. The code can
be made to read and write addresses that are at least partially
attacker controlled, but the decoder is complex and I don't have the
time to investigate fully whether the level of control is sufficient
to lead to code execution. My gut feeling is that someone more skilled
than I could cause arbitrary code execution, but I cannot say for
certain.

This bug can be used to crash bsdtar and other programs that use
libarchive, such as file-roller.

I have attached some test cases that demonstrate this.

They run as follows:
xxd -r testcase.rar.txt testcase.rar
bsdtar -Oxf testcase.rar

The test cases are:

 - oob-read.txt - Ppmd7_DecodeSymbol does an out-of-bounds read and
   crashes. (No UAF.)

 - uaf-read.txt - this heap UAF causes an out-of-bounds read in
   Ppmd7_DecodeSymbol and crashes.

 - double-free.txt - this test case causes a double-free

 - uaf-rw.txt - this shows reads and writes into a previously freed
   heap region.

I've tested all of these on the version of bsdtar that ships with
Ubuntu 18.04 Bionic and also with a build of libarchive from git. My
analysis of their behaviour comes from running them under valgrind and
ASAN. If you have any trouble reproducing them let me know.

The crashes were found with afl-fuzz and the FairFuzz extension.

I've also reported this to the OSS-Fuzz contacts for the upstream project.

[Seth Arnold (seth-arnold)]

Hi Daniel, very nice findings. Have you had any feedback from upstream authors yet?

How did you manage the CRC? Did you defeat that in the sources or did you compute 'correct' CRCs for the inputs? Is this work that can be fed to Hanno or the upstream authors to encourage far wider use of fuzzing?

Have you had sufficient core time to get a feeling for how hardened this project is?

Thanks

[Daniel Axtens (daxtens)]

Hi Seth,

 - I haven't heard anything from upstream yet.

 - I modified the sources to disable the CRC checks, and then I manually fixed up the CRCs on the files I attached so they work with an unmodified bsdtar.

   What I plan to do after the fixes land is to propose a PR that makes disabling the CRC checks for RAR an option like it is for the zip decoder, and then propose a change to the OSS-Fuzz wrapper to turn on these options.

 - All I've done is some fuzzing and run cppcheck; I haven't done any source code analysis. The results are probably above average, but not exactly encouraging:

    * I think there's an infinite loop in the WARC decoder. (I was going to report this after these fixes land.)
    * There's a NULL-deref in the ACL handling code. I'm pretty sure this is the bug described in https://www.usenix.org/system/files/conference/usenixsecurity18/sec18-yun.pdf - and I was able to reproduce it. If so, it was apparently reported upstream - probably last year - and hasn't been fixed.
    * Ubuntu's Bionic package doesn't have the fix in commit 51d7afd3644fdad725dd8faa7606b864fd125f88 upstream. This is a DoS that was found by OSS-Fuzz but seems not to have a CVE.
    * cppcheck reports an issue in the CAB decoder which I haven't specifically tried to reproduce.
    * The XAR and mtree formats are essentially untestable with regular fuzzing.

    On the other hand, unrar-free also crashes instantly on fuzzing with CRCs disabled, and unar doesn't even get through the corpus I generated fuzzing libarchive - it crashes in the RPM decoder. (These are both read-only segfaults, and the packages live in universe, so I was going to just report them to debian when I get a moment.)

   So in short, if I was opening an untrusted archive on Ubuntu, libarchive would be my pick, but my confidence tails off with rarer formats.

Regards,
Daniel

[Daniel Axtens (daxtens)]

Hi Seth,

Still no word from upstream.

Would it be helpful if I prepared patches for the issues that I've found? I can attach them here.

Regards,
Daniel

[Seth Arnold (seth-arnold)]

Daniel, that would be extremely helpful. Thanks!

[Daniel Axtens (daxtens)]

Hi Seth,

Please find attached:

 - Patch 1 - fix the double free where you could end up calling realloc(area, 0) and then free(area). This fixes the double free test case.

 - Patch 2 - (this is the one that delayed the patch set so long) when a file reports that it is split across multiple volumes, make sure the header for the next part of it matches the file name so as not to invalidate the ppmd context. This fixes the other 3 test cases.

 - Patch 3 - fix the NULL deref in ACL code

 - Patch 4 - fix the WARC infinite loop

They pass 'make check' and I'm doing a fuzzing run to make sure they haven't broken anything else. I will let you know if I find anything.

Regards,
Daniel

[Seth Arnold (seth-arnold)]

Hi Daniel, I haven't had any replies to an email I sent Tim Kientzle a few days ago. If neither of us hear anything in the next few days I think we'll just need to file some public issues. These patches looked good to me, though.

Thanks

[Daniel Axtens (daxtens)]

Hi Seth,

OK, I'm happy to do a pull request with upstream, just let me know when.

Regards,
Daniel

[Seth Arnold (seth-arnold)]

On Thu, Dec 06, 2018 at 11:40:29PM -0000, Daniel Axtens wrote:
> OK, I'm happy to do a pull request with upstream, just let me know when.

Hi Daniel, I haven't received any replies yet.

Please make your pull requests when convenient, paste the links in here,
and make the bug public.

Thanks again

[Daniel Axtens (daxtens)]

Hi Seth,

I've pushed them to https://github.com/libarchive/libarchive/pull/1105

Thanks for walking me through the Ubuntu process.

Regards,
Daniel

[Ubuntu Foundations Team Bug Bot (crichton)]

The attachment "0001-Avoid-a-double-free-when-a-window-size-of-0-is-speci.patch" seems to be a patch. If it isn't, please remove the "patch" flag from the attachment, remove the "patch" tag, and if you are a member of the ~ubuntu-reviewers, unsubscribe the team.

[This is an automated message performed by a Launchpad user owned by ~brian-murray, for any issues please contact him.]

[Daniel Axtens (daxtens)]

Here's a test case for the NULL pointer dereference in ACL handling.

[Daniel Axtens (daxtens)]

(as with the other test cases, it's in plain text, convert it back with xxd -r aclcrasher.txt aclcrasher)

[Daniel Axtens (daxtens)]

This is the warc infinite loop test case. Unlike the other files, it's *not* encoded, and I use ./bsdtar -Oxf warcloop.warc to see the looping behaviour.

[Sebastien Bacher (seb128)]

Looks like those fixed landed now, https://people.canonical.com/~ubuntu-security/cve/2018/CVE-2018-1000877.html