Created squashfs-tools tracking bugs for this issue:

Affects: fedora-all [bug 1234888]

While the code in question is in Phillip's branch, note that this issue is being reported against an upstream version of squashfs-tools that has diverged from the one we use.

The patch with the final set of fixes looks reasonable at first glance and we care carry it as a patch. But given they haven't been keeping up with changes Phillip has made since 4.3 was released, I don't want to switch over to using them as an upstream (at least not at this time).

The upstream patch really should have fixed the trace statement output as well. I don't think there is a security issue with the output formats being too narrow, but the output could be confusing. Casting to unsigned long is probably the most portable way to print size_t. The 'z' modifier is too new. I did find some comments that said it is possible for size_t to be wider than unsigned long, but this seemed rare and as long as the value to be printed fit in an unsigned long, the output would still be OK.

squashfs-tools-4.3-11.fc22 has been pushed to the Fedora 22 stable repository.  If problems still persist, please make note of it in this bug report.

(In reply to Bruno Wolff III from comment #2)

> The patch with the final set of fixes looks reasonable at first glance and
> we care carry it as a patch. But given they haven't been keeping up with
> changes Phillip has made since 4.3 was released, I don't want to switch over
> to using them as an upstream (at least not at this time).

That's good to know because I'm extremely unhappy with sasquatch.  Whilst it's aims are laudable, it simply promulgates all the bad practices inherent in the hacked and extremely poor vendor specific implementations which have plagued Squashfs for years.

Many of these hacked implementations are full of bugs, and will make Unsquashfs extremely unstable...  In addition Squashfs has a specific filesystem layout, and divergences from that are treated as errors and possible security attack vectors (trying to make Squashfs do something wrong).  To move to a non-deterministic "guess work" system where filesystem differences are merely treated as "hints" it's been generated by a "vendor variant" makes a mockery of any security checks, and the two years of security hardening I put into Squashfs-tools 4.3.

In otherwords you can either make Squashfs-tools secure from attacks by carefully checking the filesystem is correct, OR you can make Squashfs flexible and tolerant of hacked up vendor messes by using kludges and guess work.  You can't do both.

I think it is outrageous a distribution would even consider moving from the official tools to an entirely unofficial and unendorsed fork.  To make myself clear it is not simply unendorsed, I publically denounce it now.

Phillip, I'm glad you commented here. I was wondering if you were aware of the CVE as it didn't seem to be reported at sourceforge. While the bug does seem to plausibly apply to your version of squashfs, I wasn't sure if there were earlier checks that would keep the data from being out of range at the point where the patch changed things.

(In reply to Bruno Wolff III from comment #6)
> Phillip, I'm glad you commented here. I was wondering if you were aware of
> the CVE as it didn't seem to be reported at sourceforge. While the bug does
> seem to plausibly apply to your version of squashfs, I wasn't sure if there
> were earlier checks that would keep the data from being out of range at the
> point where the patch changed things.

No, this isn't checked in Unsquashfs.

Compared to the Squashfs kernel code and the Mksquashfs code-base Unsquashfs has fewer sanity checks.

This is for two reasons: one simply a matter of prioritisation, and the other due to the expected use-case.

The prioritisation issue should be simple to understand.  In terms of vulnerability the kernel is most critical, then Mksquashfs and last Unsquashfs, and the hardening and addition of sanity checks have been prioritised in that order.

The expected use-case is a little less obvious.  With the kernel code hardening, it has become increasingly impossible to mount a corrupted Squashfs filesystem.  Given we would like to allow users to extract the good data in a corrupted filesystem using Unsquashfs, it stand to reason that Unsquashfs by definition needs to be more tolerant of filesystem corruption and have less sanity checks.

Unfortunately this means Unsquashfs is more vulnerable to CVEs than the other code.

The question here is what to do about this raised CVE, and what to do in the future.

With regards to this CVE, the posted patch is a band-aid, it fixes the symptom but not the cause.  The cause is a corrupted fragments superblock entry, the symptom is this value unchecked then leads to a variable overflow and a stack overflow.  The fix isn't to paper over the variable overflow and the stack overflow, this will probably simply lead to incorrect behaviour later on.

On the one hand the correct fix is obvious, and that is to introduce more sanity checks of the superblock information.  But on the other hand this goes against the idea that Unsquashfs is supposed to be more tolerant of filesystem corruption.

The ultimate fix in the future is likely to mean making Unsquashfs more actively tolerant of filesystem corruption, rather than at the moment turning a blind eye to filesystem corruption and hoping for the best.  What this means is Unsquashfs will need to actively detect filesystem corruption, and rather than simply aborting, try to actively work around the known filesystem corruption.

To do this completely is a big task.  In the meantime I will work on a more focused patch for this CVE that fixes the cause and not the symptom.

squashfs-tools-4.3-11.fc21 has been pushed to the Fedora 21 stable repository.  If problems still persist, please make note of it in this bug report.