wc_sdk: cycle cap on IsamDeleteVariableRec + SECURITY.md

Audit pass over the vendored Wildcat! 4 SDK (~11 K LoC, circa 2003).
Most findings are either defended at the wrapper layer already
(mb.fmt.wildcat clamps MsgBytes) or live in DOS-era 16-bit segment
math that is unreachable under 32/64-bit FPC.  One credible DoS did
warrant a direct patch:

vrec.pas (IsamDeleteVariableRec): the repeat-loop walks a
NextRefNr chain with no cycle guard.  A corrupted .MDF that points
NextRefNr back at an earlier record caused the deleter to loop
forever, deleting the same records repeatedly.  Added a 10 M hop
cap matching the pattern used in the Squish ReIndex fix.

SECURITY.md: documents the threat model for the vendored code,
what is defended (MsgBytes clamp, O_NOFOLLOW on lock file, new
cycle cap), and what is not (arbitrary-offset Seek via
UserConfData, tree-walks below the variable-record layer,
FileOpen without O_NOFOLLOW on data files).  Consumers get
deployment guidance: run Wildcat under an isolated account, mount
externally-sourced bases read-only in a sandbox.

src/wc_sdk/SECURITY.md

@@ -0,0 +1,65 @@
+# wc_sdk security posture
+
+`src/wc_sdk/` is ~11 K lines of Wildcat! 4 SDK code vendored from a
+release circa 2003.  It predates modern threat models and contains
+constructs that would not pass code review today:
+
+- Fixed-size stack / static buffers filled from `BlockRead` where the
+  length comes from a field in the record being read.
+- Tree / chain walks over in-file pointers with no cycle detection.
+- `FileOpen` without `O_NOFOLLOW` on POSIX.
+- Turbo Pascal-era `New` / `Dispose` pairs whose error paths leak on
+  failure.
+
+## Threat model
+
+The attacker supplies the bytes on disk for one or more of
+`*.MDF`, `*.MNX`, `*.MKY`, the Wildcat user database, and the
+`MAKEWILD.DAT` config.  The code is invoked read-only through
+`src/formats/mb.fmt.wildcat.pas` (our wrapper) from tools that mount
+a Wildcat base -- typically a dedicated mailer / tosser account that
+should not be root.
+
+## What is defended
+
+- `mb.fmt.wildcat.TWildcatBase.ReadBody` clamps the attacker-controlled
+  `Hdr.MsgBytes` against the fixed-size `TMsgText` so the wrapper can
+  never hand the SDK a length that would overflow the receive buffer.
+- `mb.lock.TMessageLock` uses `O_NOFOLLOW` for the per-base sentinel,
+  so the lock file cannot be redirected by a symlink swap even if the
+  Wildcat directory is world-writable.
+- `vrec.IsamDeleteVariableRec` has a hop cap so a circular
+  `NextRefNr` chain in a crafted `.MDF` cannot loop the deleter
+  forever.
+
+## What is NOT defended (known residual risk)
+
+- **Arbitrary-offset Seek** via `UserConfData` in the user DB:  a
+  corrupted `USERS.XDB` can point `UserConfData` anywhere in the
+  file.  The subsequent `BlockRead` is into a fixed-size local,
+  which contains the record the parser expected to see there.  The
+  caller (`LookupUserId`) treats a bad read as a miss.  No known
+  memory-corruption primitive, but the returned data is
+  attacker-controlled.  Callers that trust user-DB fields for
+  authentication / capability checks should not rely on Wildcat
+  alone.
+- **Tree walks in `btfileio.pas` / `btisbase.pas`** have no depth or
+  cycle cap below the variable-record layer.  Malformed B-tree files
+  can theoretically hang reads.  We have not seen it in practice.
+- **`FileOpen` in `btfileio.pas`** does not pass `O_NOFOLLOW` when
+  opening data files.  Symlink-in-data-dir attacks are therefore
+  possible if the Wildcat directory is shared with an untrusted
+  local user.  Running Wildcat under a dedicated account with a
+  non-shared data directory is the recommended mitigation.
+
+## Guidance for consumers
+
+- Treat Wildcat bases exactly like any other untrusted input: run
+  the tosser / reader as an isolated account, with the data
+  directory owned by that account and mode 700.
+- If you are importing a Wildcat base from an external source,
+  mount it read-only in a sandbox (container, user namespace) the
+  first time you read it.  The hardening above bounds allocation
+  and CPU use, but a crafted base can still return garbage data --
+  that is a correctness decision for the caller, not a safety
+  guarantee from the library.

src/wc_sdk/vrec.pas

@@ -316,11 +316,20 @@ end;
 
 procedure IsamDeleteVariableRec(IFBPtr : IsamFileBlockPtr;     {!!.50}
                                   RefNr  : LongInt);
+const
+  { Cycle guard.  A corrupted data file can point NextRefNr back at
+    an earlier record in the chain; without a cap the repeat-loop
+    deletes records in a ring forever, corrupting state and burning
+    CPU.  10 M hops is far beyond any legitimate chain in a Wildcat
+    base (variable-record chains are typically < 100 entries).  }
+  MAX_CHAIN_HOPS = 10 * 1000 * 1000;
 var
   NextRefNr : LongInt;
   S         : Word;
+  Hops      : LongInt;
 begin
   S := ILI (IFBPtr^.DIDPtr^[0]^.LenRec).Lo;
+  Hops := 0;
   repeat
     IsamGetBlock(IFBPtr^.DatF, LongInt (S) * RefNr + LongInt (S-4),
                    SizeOf (LongInt), NextRefNr);
@@ -328,6 +337,12 @@ begin
     IsamDeleteRec(IFBPtr, RefNr);                              {!!.50}
     if not IsamOK then Exit;
     RefNr := NextRefNr;
+    Inc(Hops);
+    if Hops > MAX_CHAIN_HOPS then begin
+      IsamOK := False;
+      IsamError := 10415;    { reuse "record too large" error code }
+      Exit;
+    end;
   until NextRefNr = 0;
 end;