open
https://gitlab.synchro.net/main/sbbs/-/issues/1208
### Summary
The "search title forward/backward" commands (`>` and `<`) in both the sub-board reader and the mail reader decide whether two messages share a title by comparing the **16-bit subject CRC** stored in the `.sid` index, and act on that comparison **without ever verifying the actual subject string**. A CRC-16 collision therefore does not cost a wasted lookup, it produces a wrong answer: the reader silently jumps the user to a message with an entirely unrelated title.
### Where
`idxrec_t.subj` (`src/smblib/smbdefs.h:390`) is a `uint16_t` holding `smb_subject_crc()` (`src/smblib/smbhash.c:372`), which strips leading `RE:`, lowercases, trims, and returns `crc16()` of the result.
Four sites compare it raw and act on the result:
| File | Line | Command |
|---|---|---|
| `src/sbbs3/readmsgs.cpp` | 1536 | `>` search title forward (sub-board) |
| `src/sbbs3/readmsgs.cpp` | 1548 | `<` search title backward (sub-board) |
| `src/sbbs3/readmail.cpp` | 559 | `>` search title forward (mail) |
| `src/sbbs3/readmail.cpp` | 570 | `<` search title backward (mail) |
All four are the same shape, e.g. `readmsgs.cpp:1535`:
```cpp
case '>': /* Search Title forward */
for (u = smb.curmsg + 1; u < smb.msgs; u++)
if (post[u].idx.subj == msg.idx.subj)
break;
```
The matched message becomes `smb.curmsg` directly. Nothing re-reads the header to confirm `msg.subj` actually matches.
### Why this is a defect and not just a hash-quality nit
Elsewhere in the same files the index CRCs are used correctly, as a cheap **pre-filter** in front of an authoritative string compare. All of these read the real header and `stricmp()` the name before acting on a CRC hit:
- `readmsgs.cpp:227` -> `232` (private-message visibility)
- `readmsgs.cpp:257` -> `261` (`LP_BYSELF`)
- `readmsgs.cpp:273` -> `279` (`LP_OTHERS`)
- `readmsgs.cpp:1804` -> `1815` (`showposts_toyou`)
So the pattern the code already establishes is "CRC narrows, string decides." The four title-search sites skip the second half. They are the outliers, which is also why this is cheap to fix.
(One further raw comparison exists at `readmsgs.cpp:177`, gating visibility of **deleted** posts to their author for non-sub-ops. Narrower in effect and behind `can_view_deleted_msgs()`, but it is the same omission and worth fixing in the same pass.)
### How likely is it
16 bits over subject lines that have been lowercased and stripped of `RE:`.
For a sub with N distinct titles, the chance that a given title collides with at least one other is roughly N/65536, so a sub with a few thousand distinct titles has a few percent of its titles colliding with something. The user sees no error: `>` simply lands on the wrong message, and the correct next match is skipped.
### Suggested fix
Short term, and sufficient on its own: make the four title-search loops match the rest of the file. Treat the CRC as a candidate filter, then load the header and compare the subject with the same normalization `smb_subject_crc()` applies (skip `RE:`, case-insensitive). This is local, needs no format change, and fixes the visible bug.
Longer term, the underlying storage is worth revisiting: `idxrec_t.subj`,
`.to` and `.from` are all 16-bit, and the union arm they live in is exactly 6 bytes of the 20-byte record (`SIZEOF_SMB_IDXREC_T`). Widening them to 32-bit (and switching CRC-16 to FNV-1a-32, now available as `fnv1a32()` in `src/hash/fnv1a.h`) would also remove the false positives that currently cost
a header read in the four *correct* paths above, and would lift the 65535 cap that the same fields impose when they hold **user numbers** rather than name hashes for mail (`smbdefs.h:389` says "or user #", while the user API passes `int` everywhere else).
That is a `.sid` format change, but a tractable one: the `.sid` holds nothing authoritative. Every field in it is derived from the `.shd` headers, and `fixsmb` already truncates and regenerates the whole index from them (`src/sbbs3/fixsmb.c:184`). There is an existing version gate to hang it on, `smbhdr_t.version` / `SMB_VERSION` (currently `0x0310`, `src/smblib/smblib.c:41`).
Downgrade would not be supportable, so it would want a release note and lead time. Filing that half here only as context; the title-search fix does not depend on it.
-- *Authored by Claude (Claude Code), on behalf of @rswindell*
---
þ Synchronet þ Vertrauen þ Home of Synchronet þ [vert/cvs/bbs].synchro.net