Fix: a contact with no address was pinned as the string "null" #82

2026-09-04T20:10:48Z

paulhitt commented

2026-09-04 20:10:48 +00:00

(Migrated from gitlab.com)

The same platform difference as the vault index, in shipped code, with a
worse consequence.

saveTrust wrote "fnid": null explicitly when a contact's global
address was not yet known. Android's bundled org.json reads a JSON null
back as the four-character string "null" — a non-null value that is not
an address. What that caused on a phone:

Trust.pin compares a newly offered fnid against the pinned one and
treats a difference as a different person. "null" != "fnid:…", so the
FIRST TIME a real address arrived the contact was marked CHANGED —
the "security keys changed" warning, which is supposed to mean an
attack, firing on an ordinary first sighting. The vault's own comment
about minting a fresh device warns about exactly this shape of false
alarm.
rememberIdentity returns early once fnid != null, so the real
address was never resolved and the record stayed wrong permanently.
pinnedFnid fed "null" into key-log verification, which recomputes a
leaf against the address a contact was accepted under — every check
against the wrong subject.

Two halves, because records written the old way are already on phones:
the writer now omits the key entirely (an absent key reads as "" on both
platforms, which is what the reader always expected), and the reader
rejects the literal "null" so existing records repair themselves.

The test that matters is the one feeding the reader the literal string.
On the JVM a real JSON null reads back as "" whether the key is omitted
or not, so the plain round-trip passes with the bug intact — which is why
this survived every suite. Confirmed by restoring the old reader and
watching it go red.

Found by the sweep flagged in android!54. JSONObject.NULL is written in
exactly two places in this app, and both were wrong.

Co-Authored-By: Claude Opus 5 noreply@anthropic.com

The same platform difference as the vault index, in shipped code, with a worse consequence. `saveTrust` wrote `"fnid": null` explicitly when a contact's global address was not yet known. Android's bundled `org.json` reads a JSON null back as the four-character string `"null"` — a non-null value that is not an address. What that caused on a phone: - `Trust.pin` compares a newly offered fnid against the pinned one and treats a difference as a different person. `"null" != "fnid:…"`, so the FIRST TIME a real address arrived the contact was marked **CHANGED** — the "security keys changed" warning, which is supposed to mean an attack, firing on an ordinary first sighting. The vault's own comment about minting a fresh device warns about exactly this shape of false alarm. - `rememberIdentity` returns early once `fnid != null`, so the real address was never resolved and the record stayed wrong permanently. - `pinnedFnid` fed `"null"` into key-log verification, which recomputes a leaf against the address a contact was accepted under — every check against the wrong subject. Two halves, because records written the old way are already on phones: the writer now omits the key entirely (an absent key reads as `""` on both platforms, which is what the reader always expected), and the reader rejects the literal `"null"` so existing records repair themselves. The test that matters is the one feeding the reader the literal string. On the JVM a real JSON null reads back as `""` whether the key is omitted or not, so the plain round-trip passes with the bug intact — which is why this survived every suite. Confirmed by restoring the old reader and watching it go red. Found by the sweep flagged in android!54. `JSONObject.NULL` is written in exactly two places in this app, and both were wrong. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

paulhitt commented

2026-09-04 20:10:48 +00:00

(Migrated from gitlab.com)

assigned to @paulhitt

paulhitt (Migrated from gitlab.com) changed target branch from fix/vault-index-null-fields to main

2026-09-04 20:50:07 +00:00

paulhitt (Migrated from gitlab.com) merged commit c47533929e into main

2026-09-04 20:50:14 +00:00

paulhitt commented

2026-09-09 19:01:25 +00:00

(Migrated from gitlab.com)

changed the description

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: extant2000/flamenet-messenger-android#82