libmspack — Salvage Mode Use-After-Free in Cabinet Parser

1. Context

libmspack is the reference C library for Microsoft Cabinet (.cab) files, used directly by cabextract and several package managers and installer tools downstream. The cabextract -f flag enables salvage mode (MSCABD_PARAM_SALVAGE = 1), which tries to extract files even from structurally malformed cabinets.

I found this during a manual audit of mspack/cabd.c, focusing on the salvage path since it explicitly handles malformed input — the natural place to look for missed edge cases.

2. Salvage mode double-read

The cabinet format stores a coffFiles field in the header that points to the first CFFILE entry. The parser also computes where files should be based on the CFFOLDER entries. When these two values differ, the salvage path reads files from both locations and merges the results:

/* cabd.c ~line 460 */
if (cffile_offset != cfhead_file_offset) {
    /* Warning: cabinet has unusual file offset */
    struct mscabd_file *forig = cab->base.files;  /* save list from first read */
    int err2 = cabd_read_files(sys, fh, cab, fol, num_folders, num_files, salvage);
    /* merge both lists */
    if (forig) {
        struct mscabd_file *fend = forig;
        while (fend->next) fend = fend->next;
        fend->next = cab->base.files;  /* append second list after first */
        cab->base.files = forig;
    }
}

3. Root cause — circular list

cabd_read_files() only updates cab->base.files when it successfully links the first file into the list. If the second call fails before that point — for example, because the file offset from the header points past the end of the cabinet — it returns an error without ever touching cab->base.files. The pointer still holds the value of forig.

The merge code then executes:

fend->next = cab->base.files   →   fend->next = forig

The list now points to itself: forig → ... → fend → forig → .... A circular singly-linked list.

4. The crash path

When cabd_close() traverses the file list to free nodes, it walks the circular list:

1. Frees forig->filename and forig
2. Follows fend->next = forig — use-after-free on freed memory
3. Attempts to free forig->filename again — double-free
4. Attempts to free forig again — double-free / heap corruption

ASAN output:

ERROR: AddressSanitizer: heap-use-after-free on address 0x7bc6bffe00e0
READ of size 8 at ... thread T0
  #0 cabd_close  mspack/cabd.c:252
  #1 cabd_open   mspack/cabd.c:213

freed by thread T0 here:
  #1 msp_free    mspack/system.c:225

The crash reproduces 100% on every run with the PoC cabinet file.

5. Crafting the trigger

The PoC is a 62-byte cabinet file. The critical values:

• coffFiles = 100 (past the actual 62-byte file size)
• cffile_offset = 44 (computed correctly from CFFOLDER entries)
• One valid CFFILE entry at offset 44, with filename x

Flow: first read at cffile_offset=44 succeeds, builds a one-file list. Second read seeks to offset 100 in a 62-byte file — the seek succeeds on POSIX, the subsequent read returns 0 bytes (MSPACK_ERR_READ), cabd_read_files() returns without updating cab->base.files. The merge loop creates the circular list. cabd_close() crashes at line 252.

All four trigger conditions are attacker-controlled.

6. Secondary bug — boolean OR on error codes

There's a second, independent issue one line after the merge code:

err = err || err2;

MSPACK_ERR_READ = 3. If the first call succeeds (err = 0) and the second fails (err2 = 3), then 0 || 3 = 1 = MSPACK_ERR_ARGS. The correct error code is discarded and replaced with a different error value. The fix is err = err ? err : err2.

7. Fix and commit

Both bugs were fixed in commit c8336f2, merged by Stuart Caie. The fix checks whether cab->base.files was actually updated by the second call before attempting the merge:

/* Only merge if the second call produced a different list */
if (forig && cab->base.files != forig) {
    struct mscabd_file *fend = forig;
    while (fend->next) fend = fend->next;
    fend->next = cab->base.files;
    cab->base.files = forig;
} else if (forig) {
    /* second call produced no new files; restore original list */
    cab->base.files = forig;
}