Closed Bug 1468556 Opened 2 years ago Closed 2 years ago

BLRG-PT-18-013: DoS by Overly Large Files in MAR

Categories

(Toolkit :: Application Update, enhancement, P1)

enhancement

Tracking

()

RESOLVED FIXED
mozilla65
Tracking Status
firefox-esr60 --- wontfix
firefox63 --- wontfix
firefox64 --- wontfix
firefox65 --- fixed

People

(Reporter: jvehent, Assigned: jewilde)

References

(Blocks 1 open bug)

Details

(Keywords: sec-low, Whiteboard: [post-critsmash-triage][adv-main65-])

Attachments

(1 file, 1 obsolete file)

The MAR file format allowed to reference the same file content for several files. This could be abused by an attacker to create one large file (up to 4.3GB), and reference it several times, causing the extracted files to be larger in sum than the original MAR file.

Since the maximum size of a MAR file was 524288000 bytes (as defined in modules/libmar/src/mar_private.h) several million files of e.g. 450MB could get extracted from a single MAR archive.

Additionally, MAR offers to use BZIP2 to compress the files contained. Therefore, the size of the extracted files could be further reduced, by compressing them.

While exploiting this vulnerability requires a correctly signed MAR file, it is advised to check for overly large decompressed files and restrict the number of files in the MAR file.
Assignee: nobody → jewilde
Priority: -- → P1
Blocks: 1468544
Currently MAR files can contain files that reference the same content block repeatedly allowing a large amount of data to be written to disk from a small 450MB signed archive.

The number of files stored in a MAR file is already capped at 256, which seems low enough already. Counting bytes written out to files and making assumptions about how many is reasonable to write before ceasing also seemed like a non-optimal fix. This change solves this by restricting the way that libmar deals with file references in the MAR archive.

To keep from reading out the same content block after being referenced multiple times this patch adds a hash table to store content block indices encountered when enumerating the files in the MAR archive and uses the table to ignore subsequent references completely. Zero length files are excepted from this rule as they inherently share the preceding file's content block index.

The hash table offers shrinking and growing dynamically (by factors of 2) based on table load to offset the cost of storing the archive's file offsets and names a second time in memory. The definition for the maximum number of files allowed was moved into mar_hash_table.h from mar.h. Another added benefit from the hash table is providing a stronger hash function which solves for Bug 1468544 as well. (siphash 2-4 https://131002.net/siphash/)

References to unneeded header files were also removed as they were directly adjacent to code being modified.
Status: NEW → ASSIGNED
No longer blocks: 1468544
Disallows files from referencing the same bytes in the content blocks of a MAR
file by storing a list of structs containing a file's byte offsets and lengths.
A list was chosen since the cap of 256 files wouldn't produce considerable
overhead when extracting/reading/searching/etc through the archive.

Removing the ability for a MAR file to reference the same content block
repeatedly seems like a better solution than what was suggested in the BLRG
report. (limiting the number of files or checking for overly large
decompressed files)

Allows us to prohibit this type of file bomb while only losing an attribute
of the MAR file format that wasn't being leveraged. The fix is applied in
mar_enum_items and mar_find_item so that the manifest the updater uses is
equally safeguarded as the mar host tool.
Attachment #9021345 - Attachment is obsolete: true
https://hg.mozilla.org/mozilla-central/rev/06463d10abde
Group: toolkit-core-security → core-security-release
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla65
Flags: qe-verify-
Whiteboard: [post-critsmash-triage]
Whiteboard: [post-critsmash-triage] → [post-critsmash-triage][adv-main65-]
Group: core-security-release
You need to log in before you can comment on or make changes to this bug.