downloadBinary() writes HTTP error bodies to the destination file and reports success (no CURLOPT_FAILONERROR / no response-code check)

[DrewGei]

Environment

gup.exe FileVersion	5.42 (v5.4.2), as bundled with Notepad++ 8.9.7 x64
Defect verified in	master @ 80b6e0f (v5.4.3) — still present in latest
OS	<< Windows version, e.g. Windows 11 24H2 x64 >>
Network	Corporate forward proxy (Zscaler 6.2) performing threat inspection

Summary

downloadBinary() treats any completed HTTP transaction as a successful download, regardless of status code. When an intermediary returns a non-2xx response, the response body is written to the destination .zip path and the function proceeds as though a package had been retrieved.

In the plugin-install path this surfaces to the user as "Plugin package hash mismatched", which points the user at the package or the manifest — when the actual fault is that no package was ever downloaded.

Root cause

src/winmain.cpp, downloadBinary() (line 1006):

• The destination file is opened wb at line 1008, before the request is issued.
• CURLOPT_WRITEDATA is bound to that handle (line 1024), so libcurl writes whatever body arrives — including an error body.
• The only failure test is if (res != CURLE_OK) at line 1047. A 403 that returns a body is CURLE_OK; libcurl retrieved the response exactly as asked.
• CURLOPT_FAILONERROR is never set, and CURLINFO_RESPONSE_CODE is never queried.
  Confirmed repo-wide — none of CURLOPT_FAILONERROR, CURLINFO_RESPONSE_CODE, or curl_easy_getinfo appears anywhere under src/.

Control flow then reaches the hash check at lines 1069–1103 with an HTML document in a file named *.zip, and line 1103 reports it as a hash mismatch.

Reproduction

Any intermediary that returns a non-2xx response with a body will reproduce this. Observed with a Zscaler sandbox analysis hold on first encounter of an unseen archive:

1. Plugins Admin → install a plugin whose release asset the proxy has not previously seen.
2. The proxy answers the request to release-assets.githubusercontent.com with HTTP/1.1 403 Forbidden, Server: Zscaler/6.2, Content-Type: text/html, ~20 KB body.
3. gup.exe writes those ~20 KB of HTML to the destination .zip.
4. Dialog: "Plugin package hash mismatched", reporting the SHA-256 of the HTML page.
   Worth stressing what that 403 body actually says, because it is not a block:

We're checking this file for a potential security risk. The file you attempted to download is being analyzed for your protection. It is not blocked. The analysis can take up to 10 minutes, depending on the size and type of the file. If safe, your file downloads automatically.

So the upstream condition is "retry shortly," delivered as a terminal 4xx. The intermediary's own retry mechanism is client-side JavaScript (an escalating refreshTimes schedule backed by localStorage), which requires the hold page to be the top-level document. That excludes not just non-browser clients like this one, but also JavaScript-initiated downloads inside web applications, where the 403 lands in a hidden iframe or an XHR handler and fails silently. In every such case the only usable signal is the status code — which WinGUP discards.

The dialog from the original occurrence, for illustration:

The hash of plugin package "ComparePlus_cp_3.0.0_x64.zip" is not correct.
Expected: ed6256afcddca5715d81c48d3d1d2f35f4533cfa238320cb21d9d69ce86b5572
<> Found: a864f47d774dfdbef89dda52a5c974f33ee52ffd52965c529c9a7f74352690de
This plugin won't be installed.

ed6256af… is correct — it matches the upstream asset. a864f47d… is the hash of the intermediary's HTML page.

Evidence

Reproduced outside Notepad++ with a harness replicating downloadBinary()'s curl configuration (same UA form, FOLLOWLOCATION, no --fail). Seven GitHub release assets, ground-truth sizes and hashes established from an unproxied path. Pass B retests the identical URLs after 240 s.

Asset	Expected bytes	Pass A	Pass B (+240 s)
NX_ASCII_Database_Helper.zip	782,120	403 / 20,988 B HTML	200 / 782,120 B — match
NppRgaIsaPlugin_v1.0.0.0.zip	223,694	403 / 20,973 B HTML	200 / 223,694 B — match
MZC8051_x64.zip	956,611	403 / 20,904 B HTML	200 / 956,611 B — match
BunyanLogViewer_x64.zip	28,004	403 / 20,961 B HTML	200 / 28,004 B — match
HLASMLexer_x64.zip	55,528	403 / 20,913 B HTML	403 / 20,919 B HTML
Dual-Scrollbars-v1.0.0-x64.zip	52,394	403 / 20,991 B HTML	403 / 20,979 B HTML
ComparePlus_cp_3.0.0_x64.zip (control — previously fetched, cached verdict)	2,653,191	200 — match	200 — match

Four assets flip 403 → byte-exact 200 at the 240 s retest with no change other than elapsed time. The remaining two (HLASMLexer_x64.zip, Dual-Scrollbars-v1.0.0-x64.zip) also returned byte-exact 200 on a later attempt, having taken longer than the 10-minute maximum the hold page states. All six held assets ultimately downloaded correctly and matched their manifest hashes. The 403 bodies were therefore transient refusals, not corrupt packages, and no package in this set was ever actually blocked. The control confirms transport and manifest hashes are sound.

Every 403 body begins:

<!--# Id: security.html ... --> <!DOCTYPE HTML ...>
<html> <head> <meta name="description" content="... protecting their employees from malware ...">

That is what currently lands on disk as a plugin package. No Retry-After header is present on any response.

One further detail relevant to the error message: the hold page body differs on every request. Repeated fetches of the same URL returned 20,913 / 20,919 / 20,907 bytes with a different SHA-256 each time, presumably from an embedded per-transaction identifier. So the hash reported in the "Found:" field is not stable across attempts — a user comparing it between retries sees it change, reinforcing a "corrupted package" reading of what is actually a non-package.

Impact

1. Misdiagnosis. The error text sends users toward the plugin author, the manifest, or nppPluginList.dll, none of which are at fault. The information needed — an HTTP status — is available and discarded.
2. A retryable condition is rendered permanent. The upstream state was "analysis in progress, retry shortly." Because the status is never examined, WinGUP cannot distinguish it from a genuine failure, so it cannot retry and instead reports corruption. This is the practically significant consequence — inspecting proxies are common in corporate environments, and plugin installation is simply broken behind them on first fetch of any given asset.
3. The manifest hash is the sole backstop on the plugin path. It works, but it is doing integrity verification's job and transport error detection's job, and can only report failure in integrity terms. (The application-update path additionally performs Authenticode verification via verifySignedfile.cpp, so this is not a code-execution report.)
4. Not proxy-specific. Captive portals, ISP interception, GitHub 5xx, rate limiting, and expired pre-signed asset URLs all produce the same class of failure.

Proposed fix

Set CURLOPT_FAILONERROR so libcurl fails the transfer on 4xx/5xx:

curl_easy_setopt(curl, CURLOPT_FAILONERROR, 1L);

Or, to keep the status available for reporting, retrieve it explicitly after curl_easy_perform():

long httpCode = 0;
curl_easy_getinfo(curl, CURLINFO_RESPONSE_CODE, &httpCode);
if (httpCode < 200 || httpCode >= 300)
{
    // report the status; do not treat the body as a package
}

Because the destination file is opened before the request, it should also be closed and removed on this path so a partial or error body is not left behind for a later step to consume.

Two cheap defensive additions, given a manifest hash is already available:

• Reject Content-Type: text/html when an archive is expected (CURLINFO_CONTENT_TYPE).
• Verify the ZIP local-file-header magic (50 4B 03 04) before hashing. A 20 KB HTML document in place of a 782 KB archive is detectable without any network knowledge.
  Optional, and I'd understand declining it: once the status is known, a bounded retry on the transient classes (202, 429, 503, and in practice 403 from an inspecting intermediary) would let installs succeed unattended behind a scanning proxy. In my testing every held asset eventually became available, so retry would have resolved all of them — though the wait exceeded 10 minutes in two cases, which is a long time to hold an install open. I'd still rank this below the correctness fix: reporting the status accurately is the fix, and retry policy arguably belongs to the caller rather than to downloadBinary().

Secondary: distinguish download failure from integrity failure

Independently of the above, the line 1103 dialog should not describe a failed download as a hash mismatch. Two distinct conditions deserve two messages:

• Download failed — non-2xx status, wrong content type, or non-archive payload. Report URL, HTTP status, content type, and bytes received.
• Integrity mismatch — a genuine archive was retrieved whose hash differs from the manifest. This is the tampering/staleness case the current message is written for.
  Reporting the HTTP status in the first case would have made this self-diagnosing.

[rdipardo]

Good work.

The existing behavior has often tricked users into believing a plugin was not correctly packaged when in fact it simply could not be downloaded, e.g.,

• Compare plugin 2.02 is returning a hash error when you try to install from plugin admin  notepad-plus-plus#14490 (comment)
• Update PreviewHTML repo links and id nppPluginList#794