ipxe/AGENTS.md

# iPXE — agent guidance

iPXE is a network bootloader that runs pre-OS, in ring 0 / UEFI boot
services, frequently in Secure-Boot-signed builds. A memory-safety defect
in code that parses attacker-supplied data is therefore a pre-boot
code-execution / Secure-Boot-bypass primitive, not merely a crash. Review
standards are correspondingly high.

This file is tool-neutral: any coding agent should follow it. Claude Code
users additionally have `CLAUDE.md` and the `ipxe-security-review` skill,
which point back here.

The authoritative description of the threat model, the `FILE_SECBOOT`
scope semantics, the ring-0 / Secure-Boot stakes, and the exclusions
(malicious hardware, USB, UEFI peers, Infiniband) lives in
`doc/threat_model.dox`. **Read it first.** The bounds contracts of the
core helpers live **at the source**, in Doxygen documentation blocks;
the Notes section of `doc/threat_model.dox` is the definitive index of
links to them. Both are authoritative; the notes below are operational
pointers for review work, not a second copy.

## Contributions must have a human owner

`AGENTS.md` governs all agent activity here, not just security review.
Do not open pull requests or issue reports autonomously or unattended;
anything submitted upstream must be understood, owned, and defensible by
a human contributor who has reviewed it. Per the "(Ab)use of AI" policy
in `CONTRIBUTING.md`, unsolicited or unreviewed AI-generated pull
requests and issue reports are not welcome and result in a ban. The one
sanctioned exception is a concise, verified security vulnerability
report filed via `SECURITY.md`.

## Threat model & scope

Authoritative: `doc/threat_model.dox`. Operational deltas for reviewers:

- Concrete attacker inputs to hunt: DHCP / DHCPv6, DNS, TFTP, HTTP(S)
  headers / bodies / redirects, TLS, 802.1X / EAP / EAPoL, PeerDist /
  PCCRC, and downloaded images (ELF / PNM / …). At the weaker edge levels
  the model describes: USB descriptors, inputs from consumers of the EFI
  protocols iPXE *produces* (SNP, PXE Base Code, USB I/O, download), and
  platform-firmware data iPXE consumes (UEFI `LoadOptions`, `Boot####`
  device paths, configuration tables).
- `FILE_SECBOOT ( PERMITTED )` is the priority surface. Unmarked files are
  **lower priority but still in scope** — not "out of scope". `FORBIDDEN`
  is excluded by policy, not by cleanliness.
- Check each *file's own* `FILE_SECBOOT` marker, not its directory's: a
  file outside a subsystem's directory can silently miss the marker its
  siblings carry (an ONC-RPC helper outside `net/oncrpc` did exactly this).

## Reporting conventions

- One line per finding:
  `! path/file.c:NNN  <attacker input> → <bug class>`
- `!` marks a high-confidence memory-safety defect.
- Report only **actual, reachable** defects. Do the rigorous verification
  privately; keep the emitted report terse. (Verbose, speculative,
  AI-generated reports are actively unwelcome.)
- **One defect at a time** — stop after each confirmed defect so the
  maintainer can patch it. Do not batch findings.
- **Keep findings private until fixed.** No public issue or PR may
  describe an unfixed defect. Report a confirmed defect through the
  process in `SECURITY.md` — as a GitHub Security Advisory, and an
  AI-generated report must be filed against `ipxe/aipxe`, not
  `ipxe/ipxe`. Published artefacts (commit messages, the threat-model
  doc) stay descriptive: omit historical exploitation specifics that
  carry only offensive value (e.g. a pre-fix corruption window), even
  for already-fixed defects.
- **The maintainer writes the patch.** Identify the *correct fixed
  behaviour* first: e.g. a helper named after a POSIX function owes the
  POSIX validation contract, so validation belongs in that callee.

## Verify before reporting

A sweep produces **candidates, not findings** — however a candidate is
generated (a manual pass, or a fan-out of sub-agents partitioned by
subsystem). Every candidate must be verified first-hand against the source
(field types, the specific guard present or absent, the reachable call
path) before it is reported. A confident sub-agent write-up is an input to
that verification, never a substitute. Report only what survives.

## Review procedure (per target file)

1. Confirm the file's attacker input (above) and note its `FILE_SECBOOT`
   status — `PERMITTED` is priority, unmarked is lower priority.
2. Find the wire entry point (the RX / deliver / parse function where
   attacker bytes first arrive together with a length).
3. Confirm the suspect value is attacker-controlled and the path is
   reachable from network input — not reached only through trusted
   input such as operator-configured settings or the embedded/boot
   script.
4. Trace every length / offset / count from the wire to its use. Watch for:
   - `size_t` underflow (`a - b` with `a < b` → ~`SIZE_MAX`);
   - additive overflow before a bounds check (`a + b > limit`, computed in
     `uint32_t` / `size_t`, wraps and passes — recurred across SRP/FCP,
     ELF program headers, and FIP descriptors; the correct idiom is the
     subtraction form the tree uses elsewhere:
     `if ( a > limit || b > limit - a )`);
   - 32→64-bit truncation (LP64: `int` / `unsigned int` are 32-bit,
     `size_t` / `long` 64-bit);
   - missing header / trailer accounting.
   (Signed shift into the sign bit — `1 << 31` — is now caught at build
   time by `-Wshift-overflow=2`; do not hand-hunt it.)
5. Verify each access against the *real* allocation, using the documented
   helper contracts — do not re-derive them.
6. Classify the finding against `doc/threat_model.dox` — reachable ≠
   interesting:
   - a defect exploitable only by code already at iPXE's privilege is
     uninteresting (a malformed payload handed to a ring-0 loader — `nbi`,
     `elf` — can do nothing it couldn't do by simply executing; treat as
     hardening);
   - a driver defect triggered only by DMA-capable hardware is out of
     model; a USB / EFI-peer / firmware-input defect is on the edge (fix
     defensively, but rank accordingly).
   State the classification in the report so the maintainer can prioritise.
7. Report terse; stop.

## After the fix: review the PR

The maintainer writes the patch and opens a PR; reviewing it is part of
the loop. Confirm the diff actually closes the finding (correct idiom,
ordering matched to sibling checks, no new NULL-deref or regression) **and**
that the commit message's reachability / impact claims are accurate —
inversions and typos have both slipped through and are worth catching.

## Adjacent passes (not memory-safety)

You may be asked for these; they use a different lens:

- **Correctness / UB over `PERMITTED` drivers** — malicious hardware is
  still out of model; check only correctness and undefined behaviour.
- **Missing little-endian conversions** (`cpu_to_leXX` / `leXX_to_cpu`),
  best found by intra-file inconsistency (a wire / table / register field
  converted in most sibling accesses but host-order in one). Almost always
  **latent**: iPXE's hardware targets are little-endian, and the only
  big-endian target (s390x) runs solely as a Linux executable with no
  hardware drivers — so a driver-side miss is a correctness blemish, not
  exploitable. Only core / firmware-table code reachable on s390x-linux
  (e.g. an on-disk or config-table magic compared in host order) can be
  live.

## Codebase contracts (authoritative docs live at the source)

Cross-referenced from the Notes section of `doc/threat_model.dox`. Use
them; do not re-derive:

- `include/ipxe/iobuf.h` — the `iob_*` accessors (`iob_pull`, `iob_unput`,
  …) are bare pointer arithmetic with **no** production bounds check. The
  caller must validate lengths against `iob_len()` first.
- `crypto/asn1.c` — `asn1_cursor` helpers are self-checking; walking a
  structure via the helper API is bounds-safe by construction (a parse
  error invalidates the cursor to zero length).
- `core/xferbuf.c` — `xferbuf_*` accumulation is length-checked
  (`ensure_size` before copy).
- `core/vsprintf.c` — `ssnprintf` / `vssnprintf` clamp a negative
  remaining size to zero, so `used += ssnprintf(buf+used, len-used, …)`
  is safe as `used` approaches `len`.
- `core/malloc.c` — allocators are safe against malicious sizes and return
  NULL on failure (including a zero-size request); callers must handle
  NULL.
- `include/errno.h` — the composable structured-`goto` cleanup pattern
  (the cleanup that undoes a *successful* operation sits *above* that
  operation's `err_*` label, so an unreachable cleanup statement
  immediately before the first label is deliberate future-proofing, not
  dead code).

## Patterns not to misread as bugs

- **Count-then-clamp** — a parser may return a logical length larger than
  it actually wrote, having clamped the write to the buffer size. This is
  intentional.
- **End-of-image read** — downloaded images carry a guaranteed trailing
  NUL past `image->len` (see `include/ipxe/image.h`); a one-byte read
  there is in-bounds.
- **Elided VLAs** — an attacker-sized local used only via `typeof` /
  `sizeof` may be elided by the compiler at `-Os` / `-O2` but allocated on
  the stack at `-O0`; do not conclude "unreachable" from one build.