# DF-5180 — acpi_PkgStr() heap OOB read past non-NUL-terminated ACPI_TYPE_BUFFER

- **File:** `sys/dev/acpica/acpi_package.c`
- **Severity:** Low (firmware-gated heap OOB read; usually self-limited, worst-case kernel panic; no disclosure)
- **Confidence:** certain (code path proven from strlcpy + ACPICA sources)
- **Verification:** source-confirmed only — the object comes from ACPI
  firmware (DSDT/SSDT or EC/battery-supplied data behind AML). There is no
  unprivileged runtime entry point on the audit guest (its DSDT is QEMU's
  benign one), so no guest run was attempted (`verdict.json` status
  `untested`, attempts 0). Same treatment as the sibling findings
  DF-2177/DF-2385 (source-confirmed, ACPI-gated).

## Root cause

`acpi_PkgStr()` accepts `ACPI_TYPE_BUFFER` elements
(`sys/dev/acpica/acpi_package.c:86-89`) and copies them with `strlcpy()`
(:95-98):

```c
    case ACPI_TYPE_BUFFER:
	ptr = obj->Buffer.Pointer;
	length = obj->Buffer.Length;
	break;
    ...
    /* Make sure string will fit, including terminating NUL */
    if (++length > size)
	return (E2BIG);

    strlcpy(dst, ptr, length);
```

ACPICA **guarantees NUL-termination only for Strings** — externalization
copies `String.Length + 1` bytes including the terminator
(`sys/contrib/dev/acpica/source/components/utilities/utcopy.c:276-278`) —
while a Buffer is a plain `memcpy` of exactly `Buffer.Length` bytes with no
terminator appended (`utcopy.c:288-290`).

DragonFly's `strlcpy()` (`sys/libkern/strlcpy.c:56-61`) is only safe for
real C strings: when no NUL is found within the first `siz-1` bytes it
finishes with an **unbounded scan** to compute the source length:

```c
	if (n == 0) {
		if (siz != 0)
			*d = '\0';		/* NUL-terminate dst */
		while (*s++)
			;
	}
```

`siz` here is `Buffer.Length + 1`, so the copy loop reads at most
`Buffer.Length` bytes (in bounds), but if the buffer contains **no 0x00
byte**, the very first byte of the scan loop — `ptr[Buffer.Length]` — is
already **one past the end of the ACPI buffer**, and the scan runs until it
stumbles on a zero byte.

## Why it is (usually) contained — and when it is not

The external `ACPI_OBJECT` blob handed to `AcpiEvaluateObject()` callers is
one allocation, zeroed up front
(`sys/contrib/dev/acpica/source/components/utilities/utalloc.c:527`), and
element data regions are laid out back-to-back with 8-byte rounding
(`utcopy.c:285-286`; size accounting in
`sys/contrib/dev/acpica/source/components/utilities/utobject.c:512-514` adds
exactly `Buffer.Length`). Consequences:

- `Buffer.Length % 8 != 0`: the 1–7 padding bytes after the content are
  still zero from the initial memset → the scan stops inside the same
  allocation. Still a read past the logical object, but no fault.
- `Buffer.Length % 8 == 0` **and** the buffer is the last data in the
  package: the content ends **exactly at the end of the kmalloc'd block**
  → `ptr[Buffer.Length]` is the first byte past the heap allocation. The
  scan then walks adjacent heap (kmalloc/zone metadata, neighboring chunks)
  until a zero byte; if the block ends at an unmapped boundary (slab/page
  edge), the read faults → **kernel panic**.

No information is disclosed: the `strlcpy()` return value is discarded and
the write to `dst` is bounded by the checked `size`. Impact is therefore a
(best-case silent / worst-case panicking) heap OOB read.

## Reachability

Callers: `acpi_cmbat.c:374-380` extracts `_BIF` battery strings
(model/serial/type/oeminfo) with `acpi_PkgStr()`. The helper deliberately
accepts `ACPI_TYPE_BUFFER`, so any DSDT/SSDT (or EC/battery AML data)
delivering those elements as a Buffer with no zero byte reaches the scan.
Threat model: crafted ACPI tables (VM builder, firmware compromise) or
hostile embedded-controller/battery firmware — the same gate as the known
DF-2177/DF-2385 trio, which this file's pass-1 findings established as
in-scope (this is a *distinct* bug: not the NULL `ptr`, not the unbounded
`idx` — the unbounded *scan* after a well-formed buffer).

## Fix

`fix.diff` in this directory — for Buffers, bound the copy to the buffer:
find the first NUL with `memchr(ptr, 0, Buffer.Length)` and `memcpy()` that
many bytes (`dst` is already zeroed at acpi_package.c:79, so termination is
in place); Strings keep the `strlcpy()` path. Not applied to `sys/`;
not guest-validated (`fix_status: not_testable` — firmware-gated).

## Artifacts

- `VERDICT.md` — this narrative
- `trigger.dsl` — DSDT snippet that reaches the scan (documentation)
- `fix.diff` — the bounded-copy fix
- `manifest.json`, `verdict.json`