Skip to content

ZIPFile.cpp — WriteFile return value silently discarded on two paths

Summary

ZIPFile.cpp calls ::WriteFile() on two paths and discards the boolean return value. A failed write is only partially detected via the nWritten output parameter, which doesn't capture all failure modes (e.g., ERROR_DISK_FULL can set nWritten = 0 while returning FALSE, so the nWritten check does catch that case — but ERROR_INVALID_HANDLE, ERROR_ACCESS_DENIED, and async I/O errors set nWritten undefined and are missed entirely).

Locations

srchybrid/ZIPFile.cpp:

// Line 432-435 — compressed data path: return value discarded
DWORD nWritten;
::WriteFile(hFile, pBufferOut, nWrite, &nWritten, NULL);  // return value lost
if (nWritten != nWrite)
    break;

// Line 451 — uncompressed pass-through: return value discarded, nProcess reused
VERIFY(::ReadFile(m_pZIP->m_hFile, pBufferOut, nChunk, &nProcess, NULL));
if (nChunk != nProcess)
    break;
::WriteFile(hFile, pBufferOut, nChunk, &nProcess, NULL);  // return value lost
if (nChunk != nProcess)
    break;

On line 451, nProcess is the same variable used for ReadFile — it gets overwritten by WriteFile — but only after the ReadFile check, so that part is correct. The issue is the discarded WriteFile return.

Fix

// Line 432-435:
DWORD nWritten;
if (!::WriteFile(hFile, pBufferOut, nWrite, &nWritten, NULL) || nWritten != nWrite)
    break;

// Line 451:
DWORD nWritten2;
if (!::WriteFile(hFile, pBufferOut, nChunk, &nWritten2, NULL) || nChunk != nWritten2)
    break;

Using a separate nWritten2 variable on the second path also avoids the confusing reuse of nProcess across two different I/O operations.

Acceptance Criteria

  • [x] Both WriteFile return values are checked
  • [x] A write error (any error code, not just incomplete write) causes the extraction loop to break and the function to return failure
  • [x] nProcess variable is not reused across ReadFile + WriteFile in the same block