From 69ed41280fc454451e6b2d123745a917ae8a9e55 Mon Sep 17 00:00:00 2001 From: Paulus Schoutsen Date: Sun, 5 Jul 2026 17:19:36 -0400 Subject: [PATCH] Backport 5 fixes already live in the website's vendored copy These fixes were made in the opendisplay.org website's vendored copy of nrf_web_tools.js but never flowed back upstream to this standalone repo. Backporting them here so both copies match. 1. XSS: DFU error page built HTML via innerHTML interpolating data.message/data.details (a crafted DFU-zip manifest filename could inject markup). Now set via textContent. 2. HCI sequence-number reset on ACK timeout: waitForAck reset hciSequenceNumber = 0 mid-transfer, desyncing the reliable-transport counter so one lost ACK could wedge the rest of the transfer. Removed. 3. ACK-listener race: sendPacket wrote before arming waitForAck, so an ACK landing immediately after the write could be dropped. Arm the ack promise before writing. 4. disconnectPort() threw before closing: called releaseLock() on ReadableStream/WritableStream (which have no such method), so port.close() never ran and this.port went stale, blocking reconnect. Rewrote with per-step try/catch and a finally that always nulls fields. 5. Cancelled-picker misdetection: checked e.message for 'NotFoundError', but a cancelled port picker throws a DOMException carrying that in e.name. Check e.name instead. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_012g2e8mr132vcizx92WsgiR --- nrf_web_tools.js | 73 +++++++++++++++++++++++++++++------------------- 1 file changed, 44 insertions(+), 29 deletions(-) diff --git a/nrf_web_tools.js b/nrf_web_tools.js index 925f6ae..d3f483d 100644 --- a/nrf_web_tools.js +++ b/nrf_web_tools.js @@ -274,10 +274,17 @@ class NrfWebTools { content.innerHTML = `
⚠️
-

${data.message || 'An error occurred during the update process.'}

- ${data.details ? `

${data.details}

` : ''} +

+ ${data.details ? `

` : ''}
`; + // Set via textContent so error text (which can include zip-manifest + // filenames from a loaded package) cannot inject markup into the DOM. + content.querySelector('.nrf52-dfu-error-message').textContent = + data.message || 'An error occurred during the update process.'; + if (data.details) { + content.querySelector('.nrf52-dfu-error-details').textContent = data.details; + } actions.innerHTML = ` ${data.retry ? `` : ''} @@ -769,7 +776,10 @@ class NrfWebTools { this.resolveAck = resolve; this.ackTimeout = setTimeout(() => { this.resolveAck = null; - this.hciSequenceNumber = 0; + // Do not reset hciSequenceNumber here: sendPacket resends the same pkt + // built with the old seq, and the adafruit-nrfutil reference does not + // reset on timeout. Resetting desyncs the HCI reliable-transport counter + // so one lost ACK could wedge the rest of the transfer. reject(new Error("ACK timeout")); }, timeout); }); @@ -781,11 +791,14 @@ class NrfWebTools { let packetSent = false; while (!packetSent) { + // Arm the ACK listener before writing so an ACK that lands immediately + // after the write is not dropped by readLoop (which needs resolveAck set). + const ackPromise = this.waitForAck(); await this.writer.write(pkt); attempts++; - + try { - let ack = await this.waitForAck(); + let ack = await ackPromise; if (lastAck === null) { lastAck = ack; packetSent = true; @@ -895,25 +908,25 @@ class NrfWebTools { } async disconnectPort() { - if (this.port) { - try { - if (this.reader) { - await this.reader.cancel(); - this.reader.releaseLock(); - this.reader = null; - } - if (this.writer) { - await this.writer.close(); - this.writer.releaseLock(); - this.writer = null; - } - if (this.port.readable) this.port.readable.releaseLock(); - if (this.port.writable) this.port.writable.releaseLock(); - await this.port.close(); - this.port = null; - } catch (closeErr) { - // Ignore close errors - } + if (!this.port) return; + // ReadableStream/WritableStream have no releaseLock() (only the reader/writer + // do), so the old code threw before ever closing the port, leaving it open + // with a stale this.port and blocking the next reconnect. Close each step + // independently and always null every field in finally. + try { + if (this.reader) { + try { await this.reader.cancel(); } catch (e) {} + try { this.reader.releaseLock(); } catch (e) {} + } + if (this.writer) { + try { await this.writer.close(); } catch (e) {} + try { this.writer.releaseLock(); } catch (e) {} + } + try { await this.port.close(); } catch (e) {} + } finally { + this.reader = null; + this.writer = null; + this.port = null; } } @@ -1207,11 +1220,13 @@ class NrfWebTools { await this.disconnectPort(); // Check if this is a connection error and we have more firmware to flash - const isConnectionError = e.message && ( - e.message.includes('Failed to get ACK') || - e.message.includes('ACK timeout') || - e.message.includes('NotFoundError') - ); + // A cancelled port picker throws a DOMException whose *name* is + // 'NotFoundError' (its message is "No port selected by the user."), so + // check e.name, not e.message, to route cancellation to the reconnect path. + const isConnectionError = e.name === 'NotFoundError' || (e.message && ( + e.message.includes('Failed to get ACK') || + e.message.includes('ACK timeout') + )); if (isConnectionError && this.currentFirmwareIndex < this.firmwareQueue.length) { // Connection lost, need to reconnect