Live data from Hacker News

7-Zip: From Uninitialized Memory to Remote Code Execution

landave.io

41–50 of 121 posts

Re: 7-Zip: From Uninitialized Memory to Remote Code Execution

#41
post #38

Earlier quoted context omitted.

Of course, but I would still strongly advise against this. If you really cannot avoid implementing something like this, you should inspect the 7-Zip code in order to be 100% sure that the magic number detection in your filter is identical (or matches a superset) to the one from 7-Zip.

> you should inspect the 7-Zip code in order to be 100% sure that the magic number detection in your filter is identical (or matches a superset) to the one from 7-Zip CPP/7zip/Archive/Rar/RarHandler.cpp: #define SIGNATURE { 0x52 , 0x61, 0x72, 0x21, 0x1a, 0x07, 0x00 } CPP/7zip/UI/Common/OpenArchive.cpp: const Byte kRarHeader[] = { 0x52 , 0x61, 0x72, 0x21, 0x1a, 0x07, 0x00 }; CPP/7zip/Archive/Rar/Rar5Handler.cpp: #defi…

You missed my point. It is trivial to find out what the magic number is. What is more important though: How exactly is the magic number matched? From what you have written, one might be tempted to simply check whether a file begins with this magic number. And this would be wrong. If you take a look at the matching in CPP/7zip/Archive/Rar/RarHandler.cpp:

    Byte marker[NHeader::kMarkerSize];
    RINOK(ReadStream_FALSE(stream, marker, NHeader::kMarkerSize));
    if (memcmp(marker, kMarker, NHeader::kMarkerSize) == 0)
      m_Position += NHeader::kMarkerSize;
    else
    {
      if (searchHeaderSizeLimit && *searchHeaderSizeLimit == 0)
        return S_FALSE;
      RINOK(stream->Seek(m_StreamStartPosition, STREAM_SEEK_SET, NULL));
      RINOK(FindSignatureInStream(stream, kMarker, NHeader::kMarkerSize,
          searchHeaderSizeLimit, arcStartPos));
      m_Position = arcStartPos + NHeader::kMarkerSize;
      RINOK(stream->Seek(m_Position, STREAM_SEEK_SET, NULL));
    }
7-Zip finds the magic number if it appears within some searchHeaderSizeLimit, i.e., the file does not need to start (at offset 0) with the magic number. For example, 7-Zip will extract a RAR file which begins with [00 52 61 72 21 1A 07 00] (instead of [52 61 72 21 1A 07 00]) just fine.

Re: 7-Zip: From Uninitialized Memory to Remote Code Execution

#42
post #28
post #27

Earlier quoted context omitted.

HE-ASLR I am discussing with him right now, and I think we will get this. But honestly, I don't think we will ever see a 7-Zip with /GS or CFG. Not only would this cost about 1% in binary size, it would cost an additional 1% in runtime performance loss. Additionally, it would require compiling 7-Zip with a modern compiler like VS2017. You're just asking for too much.

> You're just asking for too much. I know it's not you saying this, but it's very strange given almost all files 7-zip will ever see are untrusted files downloaded from the internet I'd rather have it be 1% slower than be compromised!

For virtually everybody even 500% slower would be acceptable. 1% is 10 second, one-time cost, which is objectively a fool's bargain.

Re: 7-Zip: From Uninitialized Memory to Remote Code Execution

#43
post #41

Earlier quoted context omitted.

> you should inspect the 7-Zip code in order to be 100% sure that the magic number detection in your filter is identical (or matches a superset) to the one from 7-Zip CPP/7zip/Archive/Rar/RarHandler.cpp: #define SIGNATURE { 0x52 , 0x61, 0x72, 0x21, 0x1a, 0x07, 0x00 } CPP/7zip/UI/Common/OpenArchive.cpp: const Byte kRarHeader[] = { 0x52 , 0x61, 0x72, 0x21, 0x1a, 0x07, 0x00 }; CPP/7zip/Archive/Rar/Rar5Handler.cpp: #defi…

You missed my point. It is trivial to find out what the magic number is. What is more important though: How exactly is the magic number matched? From what you have written, one might be tempted to simply check whether a file begins with this magic number. And this would be wrong. If you take a look at the matching in CPP/7zip/Archive/Rar/RarHandler.cpp: Byte marker[NHeader::kMarkerSize]; RINOK(ReadStream_FALSE(stream…

Oh, I did not expect that. Yes you are right, one must be careful about these things.

Re: 7-Zip: From Uninitialized Memory to Remote Code Execution

#44

7-Zip needs to start a Go Fund Me or similar for a Code Signing certificate. They're like $69-89/year, which is expensive, but for such a popular piece of software it would be a nice safety net in case of site compromise. Too bad none of the big CAs have an Open Source/Charity program that would provide a Authenticode Certificate for use with that software.

That's surprising. I'd expect they receive more than $70 a year in donation already, so it should not be such a big issue.

AFAIK 7-zip doesn't take donations (feel free to correct me), but you could argue that the donations were going towards the author of the software to thank them for their work and not towards funding the development, so the author has no reason to feel compelled to buy a cert if they don't want to. Of course, that would all depend on the wording of the donation page, which I don't believe exists.

Re: 7-Zip: From Uninitialized Memory to Remote Code Execution

#45
post #20

Earlier quoted context omitted.

As far as I understand the bug, this is not about running untrusted code (but: a parsing error resulting in state corruption). Unless you refer to the 3rd party lib (7z), used by virus scanners, to analyse rar files. But typically "untrusted code" means code that was supplied "at runtime", not at compile time (like a lib), so e.g. if a virus scanner would actually execute a .exe to evaluate its effects, or run javasc…

> To be fair to virus scanner vendors, the only way to mitigate this kind of bug is NIH: don't use 3rd party libs, implement everything yourself. But then, of course, without bugs yourself, as well :) That is not the only way to mitigate such vulnerabilities. AV vendors have had plenty of time to work on sandboxing parts of their scan engines that have repeatedly been found to have vulnerabilities like this one. Some…

> no one

Plenty of people recommend Firefox despite it sharing processes between tabs. More than 10 years after MS sandboxed IE.

Re: 7-Zip: From Uninitialized Memory to Remote Code Execution

#46
post #7

There were some misunderstandings that I want to clear up (maybe I will add them in an update to the blog post): 1. Some people mentioned that this would "only affect RAR files" and it would be safe to extract 7z files with 7-Zip prior to version 18.05. This is wrong, because 7-Zip detects the file type from the magic numbers at the beginning of the file. So the exploit can be renamed to 'exploit.7z' and it works jus…

This includes various anti-virus software. It's fascinating that this category of equipment, which searches for viruses by running untrusted code , is still regularly installed in all corners of valuable networks.

They are not intentionally running untrusted code. But yes, anti-virus software can reduce security.

Re: 7-Zip: From Uninitialized Memory to Remote Code Execution

#47

Earlier quoted context omitted.

Modern C++ does not mandate that every value be a valid instance of its type wherever it is theoretically accessible, so I'm not sure why you're saying modern C++ fixes this. Actually, the fact that Rust does is something people somewhat regularly complain about. It is a heavy-handed performance / code complexity vs. safety tradeoff that certainly fixes this bug, which I think is a pretty reasonable thing to point ou…

If you follow C++14 core guidelines, the defect described would not have occurred. The bug at its core is usage of unsanitized input data. You don't need to "mandate that every value be a valid instance of its type wherever it is theoretically accessible", however that may differ from simple sane C++14 paradigms. OPINION ALERT: Honestly guys get over rust, it does not offer a single advantage in real-life programming…

[deleted]

Re: 7-Zip: From Uninitialized Memory to Remote Code Execution

#48

Earlier quoted context omitted.

Modern C++ does not mandate that every value be a valid instance of its type wherever it is theoretically accessible, so I'm not sure why you're saying modern C++ fixes this. Actually, the fact that Rust does is something people somewhat regularly complain about. It is a heavy-handed performance / code complexity vs. safety tradeoff that certainly fixes this bug, which I think is a pretty reasonable thing to point ou…

If you follow C++14 core guidelines, the defect described would not have occurred. The bug at its core is usage of unsanitized input data. You don't need to "mandate that every value be a valid instance of its type wherever it is theoretically accessible", however that may differ from simple sane C++14 paradigms. OPINION ALERT: Honestly guys get over rust, it does not offer a single advantage in real-life programming…

So I have to read a 21762 line document just to avoid shooting myself on the foot?

There's a Rust advantage right there.

Re: 7-Zip: From Uninitialized Memory to Remote Code Execution

#49

Is there software running on Linux which is derived from the same source and is also vulnerable? Is this package vulnerable: https://packages.debian.org/sid/p7zip-rar https://packages.ubuntu.com/bionic/p7zip-rar ?

Yes, they are. I just removed p7zip from my arch box for now. Looking at the project, I think it may take a while to get up to 18.05

Re: 7-Zip: From Uninitialized Memory to Remote Code Execution

#50
post #27
post #21

Earlier quoted context omitted.

If you can convince him to use Control Flow Guard, Stack Canaries, and HE-ALSR then you should be nominated for for whatever the security community has as an equivalent to a Nobel prize. If you can convince him to get rid of his custom garbage Stdlib replacements and use ISO C++ then you're a hero to maintainability (and would probably improve the performance because the stdlib has move support).

HE-ASLR I am discussing with him right now, and I think we will get this. But honestly, I don't think we will ever see a 7-Zip with /GS or CFG. Not only would this cost about 1% in binary size, it would cost an additional 1% in runtime performance loss. Additionally, it would require compiling 7-Zip with a modern compiler like VS2017. You're just asking for too much.

Interesting insight -- thanks!

I agree with others here on the security vs. performance and security vs. binary size trade-offs. As a thought experiment I wondered at what point it would become a hard decision if I were in his shoes. I think a 10% performance hit would make it a harder decision for me, but at that point I would probably still err on the side of caution and prioritize security.

Of course it's possible this is exactly one of the reasons Igor owns something like 7-Zip, and I don't :-)

Post reply on HN