Skip to content

ArchiveRecovery.cpp — three unchecked malloc() calls crash on OOM

Historical reference only: stale-v0.72a-experimental-clean and analysis\stale-v0.72a-experimental-clean are retired reference sources, not active branch targets or current baselines. Use them only as provenance or idea-extraction sources; landed status is determined against main. See Historical References.

Summary

ArchiveRecovery.cpp allocates three buffers inside the ACE archive header parser using malloc() but immediately uses the result without checking for NULL. On an out-of-memory condition (rare but possible on a loaded system), the code dereferences NULL, crashing the process.

Product Decision

On 2026-04-26, archive preview/recovery was explicitly retained as-is for Release 1. This surface is not a release hardening target, and this bug should not be scheduled unless that product decision is explicitly reversed.

Locations

srchybrid/ArchiveRecovery.cpp:

// Line 1082 — AVSIZE buffer
acehdr->AV = (char*)malloc((size_t)acehdr->AVSIZE + 1);
hdrread += aceInput->Read(acehdr->AV, acehdr->AVSIZE);  // crash if AV == NULL
headcrc = getcrc(headcrc, (UCHAR*)acehdr->AV, acehdr->AVSIZE);

// Line 1092 — COMMENT_SIZE buffer
acehdr->COMMENT = (char*)malloc((size_t)acehdr->COMMENT_SIZE + 1);
hdrread += aceInput->Read(acehdr->COMMENT, acehdr->COMMENT_SIZE);  // crash if COMMENT == NULL

// Line 1101 — DUMP buffer for trailing unknown data
acehdr->DUMP = (char*)malloc(dumpsize);
hdrread += aceInput->Read(acehdr->DUMP, dumpsize);  // crash if DUMP == NULL

By contrast, lines 1246 and 1262 (filename and comment for the file block) DO check the malloc return value — indicating the pattern was known but applied inconsistently.

Historical Fix Shape

Add null checks after each allocation and abort the header parse:

// Line 1082:
acehdr->AV = (char*)malloc((size_t)acehdr->AVSIZE + 1);
if (!acehdr->AV) return false;  // or break out of the parse loop
hdrread += aceInput->Read(acehdr->AV, acehdr->AVSIZE);

// Line 1092:
acehdr->COMMENT = (char*)malloc((size_t)acehdr->COMMENT_SIZE + 1);
if (!acehdr->COMMENT) return false;
hdrread += aceInput->Read(acehdr->COMMENT, acehdr->COMMENT_SIZE);

// Line 1101:
acehdr->DUMP = (char*)malloc(dumpsize);
if (!acehdr->DUMP) return false;
hdrread += aceInput->Read(acehdr->DUMP, dumpsize);

Alternatively, replace the three malloc calls with new (std::nothrow) char[...] — returns nullptr instead of throwing, and is consistent with the rest of the C++ codebase.

Historical Acceptance Criteria

  • [ ] All three malloc sites in the ACE header parser check for NULL return
  • [ ] On allocation failure: header parse aborts cleanly without null dereference
  • [ ] Existing ACE archive recovery behaviour is unchanged on non-OOM paths

Experimental Branch Decision

In stale-v0.72a-experimental-clean: ArchiveRecovery.cpp was removed entirely in commit c4cef47 (FIX: retire legacy archive recovery feature). That is not the current product decision.

Current decision: neither retirement nor hardening is planned. The feature is intentionally kept unchanged, so this item is closed as Wont-Fix.