From a785169293666b83d2022278ef0fc6935ec54cb1 Mon Sep 17 00:00:00 2001 From: Renato Alcara Date: Thu, 11 Jun 2026 00:02:14 -0300 Subject: [PATCH 1/3] fix(audit round-5): SIG-P2-NOVO readVarint shift overflow + MDB schema NOT NULL MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Round-5 audit on develop @ 2155e25065 (post-PR-#537 merge) flagged three follow-ups. All three applied here. ## SIG-P2-NOVO — `readVarint` off-by-one in the shift guard `src/Signal/libsignal.ts` The guard was `if (shift > 35)`, but JavaScript `<<` is modulo-32 on the right operand. The 5 valid byte shifts are 0, 7, 14, 21, 28; after the 5th byte the increment makes `shift = 35`. With `>`, byte 6 was still read on the next iteration and `(byte & 0x7f) << 35` silently folded into `<< 3`, producing a corrupt value that the outer `>>> 0` happily accepted as a "valid" uint32. The effect was subtle: a crafted PreKeyWhisperMessage with a 6-byte varint field tag could neutralise `extractIdentityFromPkmsg`'s field-3 (identityKey) parse — degrading the audit-SIG-A1 identity-change detection signal to undefined for that pkmsg. Decryption itself stays correct (libsignal processes the ciphertext independently); only the WARN-on-identity-change loop loses visibility. Fix: `shift >= 35` (one character). Comment rewritten to document the JS shift-mod-32 reasoning so future readers don't undo it. ## MDB-schema-raw_string — `jid.raw_string` NOT NULL `src/Utils/multi-db-sqlite/schemas/msgstore.ts` `raw_string TEXT` (nullable) had two distinct failure paths: (1) SQLite treats NULL as DISTINCT inside a UNIQUE index, so a malformed insert path producing NULL would silently bypass the `jid_raw_string_idx` uniqueness guarantee. (2) `selectJidIdByRaw` filters by `WHERE raw_string = ?`, and SQL `NULL = NULL` is FALSE — a row with raw_string NULL would never be findable again, making `rowIdFor` throw "failed to materialize jid row" for every subsequent access. `CREATE TABLE IF NOT EXISTS` is a no-op when the table exists, so legacy databases keep the nullable column (no migration risk); new databases get the constraint enforced. ## MDB-P2-A — `jid_map.sort_id` NOT NULL DEFAULT 0 `src/Utils/multi-db-sqlite/schemas/msgstore.ts` `sort_id INTEGER` (nullable) was first flagged in round-3. A row inserted without an explicit `sort_id` would end up as NULL. `ORDER BY sort_id DESC` ranks NULLs LAST in SQLite, which would silently demote a freshly-inserted-but-NULL row below older ones — the opposite of the `getAllLidsForPn` "last write wins" intent. `upsertMap` always provides a real epoch-ms tick today, so the default is purely defensive against future code paths that bypass upsertMap. ## Not applied (intentional) - **SOCK-P1** (`ev.flush()` incondicional) — toca buffer behavior that feeds the carousel/list/button render path. Per project constraint (no regressions in carousel/list/button), needs E2E coverage before any change. - **UTL-P1-A/B** (subarray aliasing + double-count fileLength) — needs benchmark/integration test before mutating HMAC stream and upload pipeline. - **MDB-P1-C** (`clear()` retry) — needs crash-recovery test. - **MSG-P1-A/B/C/D** — still deferred pending E2E, same as PR #537. ## Validation - `tsc --noEmit -p tsconfig.json` exit 0 - `eslint` on the 2 touched files: 0 errors - Local jest failures are pre-existing `better-sqlite3` native binding misses on Windows toolchain — unrelated. CI Linux is the gate. Co-Authored-By: Claude Opus 4.7 --- src/Signal/libsignal.ts | 12 +++++++++- src/Utils/multi-db-sqlite/schemas/msgstore.ts | 24 +++++++++++++++++-- 2 files changed, 33 insertions(+), 3 deletions(-) diff --git a/src/Signal/libsignal.ts b/src/Signal/libsignal.ts index 90455240875..7b15f71ac59 100644 --- a/src/Signal/libsignal.ts +++ b/src/Signal/libsignal.ts @@ -305,7 +305,17 @@ function readVarint(buffer: Uint8Array, offset: number): { value: number; nextOf } shift += 7 - if (shift > 35) { + // Reject once shift would reach 35 — NOT just exceed it. The 5 + // valid byte shifts are 0, 7, 14, 21, 28. After processing byte 4 + // (`shift` was 28), this increment makes `shift = 35`. With the + // earlier `shift > 35` guard, byte 5 would still be read on the + // next loop iteration and `(byte & 0x7f) << 35` would silently + // fold into `<< 3` because JS `<<` is modulo-32 on the right + // operand — producing a corrupt value that the outer `>>> 0` + // would happily accept as a valid uint32, neutralising the + // `extractIdentityFromPkmsg` field-3 (identityKey) parse on + // crafted pkmsg payloads. (audit SIG-P2-NOVO) + if (shift >= 35) { // Varint too long (max 5 bytes for 32-bit) // This could indicate a malformed or malicious message // Caller should log this condition at debug level diff --git a/src/Utils/multi-db-sqlite/schemas/msgstore.ts b/src/Utils/multi-db-sqlite/schemas/msgstore.ts index 62262210319..2525cfa1fb6 100644 --- a/src/Utils/multi-db-sqlite/schemas/msgstore.ts +++ b/src/Utils/multi-db-sqlite/schemas/msgstore.ts @@ -27,7 +27,20 @@ CREATE TABLE IF NOT EXISTS jid ( agent INTEGER, device INTEGER, type INTEGER, - raw_string TEXT + /* audit MDB-schema-raw_string: NOT NULL prevents two distinct failure + modes — + (1) SQLite treats NULL as DISTINCT inside a UNIQUE index, so any + malformed insert path that produced NULL would silently create + duplicate rows that the jid_raw_string_idx was supposed to + prevent; + (2) selectJidIdByRaw filters by WHERE raw_string = ?, and SQL + NULL = NULL is FALSE, so any such row would never be found + again — rowIdFor would then throw "failed to materialize jid + row" for every subsequent access. + CREATE TABLE IF NOT EXISTS is a no-op when the table exists, so + legacy databases keep the nullable column; new databases get the + constraint enforced. */ + raw_string TEXT NOT NULL ); CREATE UNIQUE INDEX IF NOT EXISTS jid_raw_string_idx ON jid (raw_string); @@ -36,7 +49,14 @@ CREATE INDEX IF NOT EXISTS jid_user_server_idx ON jid (user, server); CREATE TABLE IF NOT EXISTS jid_map ( lid_row_id INTEGER PRIMARY KEY NOT NULL, jid_row_id INTEGER NOT NULL, - sort_id INTEGER + /* audit MDB-P2-A: NOT NULL DEFAULT 0 so a row inserted without an + explicit sort_id can never end up as NULL. ORDER BY sort_id DESC + ranks NULLs LAST in SQLite which would silently demote a freshly- + inserted-but-NULL row below older ones — the opposite of the + "last write wins" intent. upsertMap always provides a real + epoch-ms tick so the default is just defensive against any future + code path that bypasses upsertMap. */ + sort_id INTEGER NOT NULL DEFAULT 0 ); CREATE INDEX IF NOT EXISTS jid_map_jid_row_id_idx ON jid_map (jid_row_id); From 52fa52ea2c98fb5c79f4ffcab92523db62eab69d Mon Sep 17 00:00:00 2001 From: Renato Alcara Date: Thu, 11 Jun 2026 00:38:10 -0300 Subject: [PATCH 2/3] fix(audit round-5): drop audit-ID tags + tighten SQL NULL wording MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Code-review feedback on PR #538: 1. The three comments each opened with an audit-ID prefix (`(audit SIG-P2-NOVO)`, `audit MDB-schema-raw_string:`, `audit MDB-P2-A:`). These rot — a reader six months from now won't recognise the IDs without spelunking through PR history. Audit cross-references belong in the commit message and PR description; the in-tree comment should explain the WHY only. Dropped all three prefixes; the technical explanations stay. 2. `// SQL NULL = NULL is FALSE` was technically wrong. In SQL, `NULL = NULL` evaluates to UNKNOWN, not FALSE. The downstream effect in a `WHERE` clause is identical (row isn't returned), but the imprecise wording would make a SQL-savvy reader pause and question the rest of the comment. Rewritten to: `SQL NULL = NULL evaluates to UNKNOWN, so the row is never returned`. No functional change — pure documentation cleanup. Co-Authored-By: Claude Opus 4.7 --- src/Signal/libsignal.ts | 14 +++++------ src/Utils/multi-db-sqlite/schemas/msgstore.ts | 23 +++++++++---------- 2 files changed, 18 insertions(+), 19 deletions(-) diff --git a/src/Signal/libsignal.ts b/src/Signal/libsignal.ts index 7b15f71ac59..52f13b2daab 100644 --- a/src/Signal/libsignal.ts +++ b/src/Signal/libsignal.ts @@ -307,14 +307,14 @@ function readVarint(buffer: Uint8Array, offset: number): { value: number; nextOf shift += 7 // Reject once shift would reach 35 — NOT just exceed it. The 5 // valid byte shifts are 0, 7, 14, 21, 28. After processing byte 4 - // (`shift` was 28), this increment makes `shift = 35`. With the - // earlier `shift > 35` guard, byte 5 would still be read on the - // next loop iteration and `(byte & 0x7f) << 35` would silently - // fold into `<< 3` because JS `<<` is modulo-32 on the right - // operand — producing a corrupt value that the outer `>>> 0` - // would happily accept as a valid uint32, neutralising the + // (`shift` was 28), this increment makes `shift = 35`. With a + // `shift > 35` guard, byte 5 would still be read on the next loop + // iteration and `(byte & 0x7f) << 35` would silently fold into + // `<< 3` because JS `<<` is modulo-32 on the right operand — + // producing a corrupt value that the outer `>>> 0` would happily + // accept as a valid uint32, neutralising the // `extractIdentityFromPkmsg` field-3 (identityKey) parse on - // crafted pkmsg payloads. (audit SIG-P2-NOVO) + // crafted pkmsg payloads. if (shift >= 35) { // Varint too long (max 5 bytes for 32-bit) // This could indicate a malformed or malicious message diff --git a/src/Utils/multi-db-sqlite/schemas/msgstore.ts b/src/Utils/multi-db-sqlite/schemas/msgstore.ts index 2525cfa1fb6..a0b1c5186a1 100644 --- a/src/Utils/multi-db-sqlite/schemas/msgstore.ts +++ b/src/Utils/multi-db-sqlite/schemas/msgstore.ts @@ -27,16 +27,15 @@ CREATE TABLE IF NOT EXISTS jid ( agent INTEGER, device INTEGER, type INTEGER, - /* audit MDB-schema-raw_string: NOT NULL prevents two distinct failure - modes — + /* NOT NULL prevents two distinct failure modes — (1) SQLite treats NULL as DISTINCT inside a UNIQUE index, so any malformed insert path that produced NULL would silently create duplicate rows that the jid_raw_string_idx was supposed to prevent; (2) selectJidIdByRaw filters by WHERE raw_string = ?, and SQL - NULL = NULL is FALSE, so any such row would never be found - again — rowIdFor would then throw "failed to materialize jid - row" for every subsequent access. + NULL = NULL evaluates to UNKNOWN, so the row is never returned + — rowIdFor would then throw "failed to materialize jid row" + for every subsequent access. CREATE TABLE IF NOT EXISTS is a no-op when the table exists, so legacy databases keep the nullable column; new databases get the constraint enforced. */ @@ -49,13 +48,13 @@ CREATE INDEX IF NOT EXISTS jid_user_server_idx ON jid (user, server); CREATE TABLE IF NOT EXISTS jid_map ( lid_row_id INTEGER PRIMARY KEY NOT NULL, jid_row_id INTEGER NOT NULL, - /* audit MDB-P2-A: NOT NULL DEFAULT 0 so a row inserted without an - explicit sort_id can never end up as NULL. ORDER BY sort_id DESC - ranks NULLs LAST in SQLite which would silently demote a freshly- - inserted-but-NULL row below older ones — the opposite of the - "last write wins" intent. upsertMap always provides a real - epoch-ms tick so the default is just defensive against any future - code path that bypasses upsertMap. */ + /* NOT NULL DEFAULT 0 so a row inserted without an explicit sort_id + can never end up as NULL. ORDER BY sort_id DESC ranks NULLs LAST + in SQLite which would silently demote a freshly-inserted-but-NULL + row below older ones — the opposite of the "last write wins" + intent. upsertMap always provides a real epoch-ms tick so the + default is just defensive against any future code path that + bypasses upsertMap. */ sort_id INTEGER NOT NULL DEFAULT 0 ); From 81fa978b5929e53723cb298f5faa9a71fd5c7500 Mon Sep 17 00:00:00 2001 From: Renato Alcara Date: Sat, 13 Jun 2026 20:32:11 -0300 Subject: [PATCH 3/3] fix(voip): add .js suffix to WAProto import in signaling/bridge.ts MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ESM strict (this package has `"type": "module"`) requires explicit file extensions on relative imports. `src/Voip/signaling/bridge.ts:36` was the ONLY file in the codebase importing `WAProto/index` without the `.js` suffix — every other ~40 import call site already uses `'../../WAProto/index.js'`. The compiled output landed in `lib/Voip/signaling/bridge.js` as `from '../../../WAProto/index'` (no `.js`). `tsc-esm-fix` patches `.js` suffixes onto relative imports that stay inside `src/`, but `../../../WAProto/index` reaches OUTSIDE the source tree and is left alone. Result: a consumer installing this fork via `npm install github:rsalcara/InfiniteAPI#develop` would crash at first require of the VoIP bridge with: Error [ERR_MODULE_NOT_FOUND]: Cannot find module '/var/www/.../node_modules/baileys/WAProto/index' imported from .../lib/Voip/signaling/bridge.js Fix is one character: `'../../../WAProto/index'` → `'../../../WAProto/index.js'`. Confirmed locally — the rebuilt `lib/Voip/signaling/bridge.js` now emits the correct `from '../../../WAProto/index.js'`. Reproduces directly: deploy the broken build, start the consumer with PM2, the VoIP module is touched during signaling init and the process dies before the WhatsApp socket reaches `connection:open`. The earlier `device_removed` 401 in the user's log is unrelated (regular session logout on a removed device). Co-Authored-By: Claude Opus 4.7 --- src/Voip/signaling/bridge.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Voip/signaling/bridge.ts b/src/Voip/signaling/bridge.ts index ec09dc43dbf..758ea8be2fc 100644 --- a/src/Voip/signaling/bridge.ts +++ b/src/Voip/signaling/bridge.ts @@ -33,7 +33,7 @@ const ACK_TIMEOUT_MS = 15_000 // version lazy-loaded `@whiskeysockets/baileys` as a peer dep. Inside the fork // we ship as part of the same package, so static imports are cleaner and avoid // the runtime `import()` ceremony. -import { proto } from '../../../WAProto/index' +import { proto } from '../../../WAProto/index.js' import { encodeWAMessage, unpadRandomMax16 } from '../../Utils/generics' import { parseAndInjectE2ESessions } from '../../Utils/signal' import { encodeSignedDeviceIdentity } from '../../Utils/validate-connection'