`pihole-FTL --config <key> <value>` puts the value in argv, which is
visible via /proc/<pid>/cmdline and `ps -eo args=`. For secret values
(notably the web password) this leaks to other unprivileged users for
the lifetime of the child process.
Allow reading the value from stdin by passing `-` in place of the
value: `echo secret | pihole-FTL --config webserver.api.password -`.
fgets reads up to 4095 bytes into a stack buffer, trailing CR/LF is
stripped, and the existing set_config_from_CLI validation pipeline is
reused unchanged.
Buffer is bounded at 4096 bytes; values that fill the buffer without
a terminating newline are rejected with a clear error rather than
silently truncated.
Signed-off-by: bcambl <5565939+bcambl@users.noreply.github.com>
Expose a cJSON-backed JSON decoder to Lua scripts via the pihole
library. Uses the thread-safe cJSON_ParseWithLengthOpts variant with a
stack-local error pointer, consistent with other FTL cJSON consumers
(teleporter.c, cli.c, http-common.c).
Includes BATS tests for round-trip, nested objects, empty containers,
and invalid-JSON error reporting.
Signed-off-by: bcambl <5565939+bcambl@users.noreply.github.com>
The API test suite asserts exact DNS query counters. Several of them
(`TOTAL`, `DNSKEY`, `DS`, `TOP_DOMAIN`) depend on how many DNSKEY/DS
lookups dnsmasq issues while validating DNSSEC. Until now those lookups
recursed to the live ICANN root, because FTL configures the real root
trust anchors whenever `dns.dnssec` is enabled and the local PowerDNS
recursor had no root zone of its own. The number of root DNSKEY queries
therefore tracked ICANN's published root key set, so an ongoing
key-signing-key rollover silently shifted the counters (9 -> 7 DNSKEY)
and broke the suite even on unrelated PRs.
We make the whole suite hermetic:
1. Serve a locally-signed root zone from PowerDNS, forward `.` to it and
trust its key, so root DNSKEY validation resolves inside the test
environment instead of reaching the internet.
2. Mark the locally-served *unsigned* zones (`icloud.com`,
`apple-dns.net`, `in-addr.arpa`, `ip6.arpa`) as local domains, so
dnsmasq no longer proves them unsigned by walking up to the real root.
3. Give the `bogus` zone a deliberately mismatched local trust anchor so
it fails validation locally rather than by failing to find a secure
delegation at the root.
4. Drop the root-key pre-warm `dig`, an internet round-trip that no
longer serves any purpose.
With no query leaving for the real root the counters are stable and
independent of ICANN key rollovers, so they are recalibrated
accordingly. Marking the extra zones as local emits the same "negative
DS reply without NS record" warning already whitelisted for `ftl`, so
the `test_final` whitelist is broadened to match it for any zone.
Signed-off-by: DL6ER <dl6er@dl6er.de>
Cover three fixes from the security pass: session cookies carry the Secure attribute only over TLS (and never over plain HTTP), an accepted TOTP code cannot be replayed, and /api/queries clamps an unbounded length parameter. The cookie and TOTP tests live in a self-contained module so they do not disturb the order-dependent workflow in test_z_auth.py; the TOTP test clears the shared secret via the session created by the accepted login so cleanup neither trips the replay guard nor emits a 2FA warning. Account for the module's four extra pihole.toml writes in test_final.bats.
Signed-off-by: DL6ER <dl6er@dl6er.de>
CivetWeb URL-decodes local_uri_raw in place, so an encoded CR/LF (%0d%0a) in the path arrived as a literal newline. redirect_lp_handler reflected that path into the Location header of a 301 redirect verbatim, before any authentication, allowing unauthenticated HTTP response-header injection / response splitting (e.g. Set-Cookie fixation). Reject any request whose decoded URI contains control characters from a begin_request hook, which runs before auth and every handler, closing the whole class of URI-into-header injection.
The rejection is logged only at debug level and never echoes the URI: begin_request runs for every unauthenticated request, so logging each one at warning level (or logging the URI verbatim) would itself be a log-flooding / log-injection vector.
Add raw-socket API tests (TestURIControlCharRejection) verifying an encoded CR/LF path is rejected with 400 and not reflected into response headers, a bare control character is rejected, and a clean .lp request is still served.
Signed-off-by: DL6ER <dl6er@dl6er.de>
Fork each case in the full-suite run so a sanitizer abort (or any crash)
fails only that case instead of taking down the rest of the suite. A
single ./tar_regression invocation now runs every case under
-DTAR_REGRESSION_SANITIZE=ON, restoring the per-case isolation of the
former Python harness without duplicating the parser and without the
need for an external --list shell loop.
The child flushes stdio before _exit() so its PASS/FAIL line is not lost
when stdout is fully buffered (e.g. captured by the bats run). The
parent reports cases that die by signal; assertion failures and ASan
_exit() errors propagate via the child exit code. Named single-case
runs stay in-process for straightforward debugging, and a failed fork()
falls back to running in-process.
Signed-off-by: DL6ER <dl6er@dl6er.de>
Replace the run-time compile driver (test/tar_regression.py) with a
proper CMake target. The harness now #includes src/zip/tar.c directly so
it can exercise the file-internal helpers (parse_tar_size,
tar_entry_span) and reuse the TAR_* layout constants, dropping the
duplicated stride calculation and constants that could silently drift
from the real parser.
The test binary builds alongside pihole-FTL, is copied to the repository
root by build.sh, and is run by the existing "TAR parser regression
harness" bats test. AddressSanitizer/UBSan instrumentation is available
via -DTAR_REGRESSION_SANITIZE=ON and is off by default so the static
musl CI builds keep linking.
Signed-off-by: DL6ER <dl6er@dl6er.de>
The API query-count assertions depend on resolving mask.icloud.com, whose
mask.icloud.com -> mask.apple-dns.net CNAME chain was recursed to the public
internet. dnsmasq fires extra DNSKEY validation queries depending on whether
Apple currently DNSSEC-signs icloud.com / apple-dns.net, and Apple toggles this
over time. The runtime DS-probing workaround in conftest.py could not reliably
model dnsmasq's behaviour (e.g. when Apple returns SERVFAIL on DS), so the suite
went flaky again.
Serve the icloud.com and apple-dns.net zones from the local authoritative
PowerDNS server instead, so the chain resolves hermetically and the query counts
are deterministic regardless of Apple's upstream DNSSEC posture. The DS-probing
fixture is dropped and the expected counters become fixed constants again.
Signed-off-by: DL6ER <dl6er@dl6er.de>
Crashes in the field are almost always one-shot and non-reproducible, so the
crash handler must extract the most useful data it can in a single pass, and
symbolization must never be able to hang or silence the handler.
Frame collection:
- Prefer walking the interrupted signal context (x86_64 RBP/RIP, aarch64
x29/PC from ucontext_t) and fall back to _Unwind_Backtrace only when that
fails. The header notes when frames came "from signal context" so field
reports show how they were obtained.
- Record frame #0 as the exact faulting PC (no -1 adjustment); the
_Unwind_Backtrace path uses _Unwind_GetIPInfo so only real return addresses
are adjusted.
- Read /proc/self/maps with raw open/read (single snapshot reused for the
whole walk) instead of fopen/fgets/sscanf, keeping the collection path
async-signal-safe and avoiding per-frame re-parsing.
- When the frame-pointer walk stops early on a corrupt or not-yet-set-up frame
pointer, cross-check it against the libgcc/_Unwind DWARF-CFI unwinder, which
can cross frame-pointer-less boundaries the walk cannot. The cross-check is
aligned to the faulting PC (so our own handler and the signal trampoline are
skipped) and printed as a secondary section only when it recovers more frames
than the truncated walk.
Symbolization:
- Run addr2line once per object (batched), so a static-pie build resolves the
whole stack in a single subprocess, spawned directly via fork/execvp (no
/bin/sh, no stdio buffering) instead of per-frame popen().
- Bound each addr2line run with a wall-clock poll() deadline; a hung addr2line
is SIGKILLed and reaped (EINTR-safe waitpid, no zombie) so it cannot wedge
the handler. No process-wide signal/timer state and no cross-thread
siglongjmp(), so it is safe in the multi-threaded daemon.
- Always print raw, re-symbolizable "addr2line -f -e <obj> <addr>" reproducer
lines for frames that stay unresolved, plus a status note explaining why
symbolization did not run (disabled / not installed / unavailable / timed
out). Fully resolved backtraces print none of this.
- dladdr() symbol fallback and a last-resort /proc/self/maps basename lookup
for frames addr2line cannot resolve.
- Render frames in gdb's style and, on an interactive terminal, color them
(function names yellow, source locations / objects green) via the existing
cli_color() helper; the daemon's FTL.log stays plain text.
Robustness:
- Per-thread _Thread_local work buffers (frame table, addr2line argv/io, maps
snapshot) so concurrent generate_backtrace() callers (e.g. lock-debug
paths) do not race, while staying off the 16 KiB alternate signal stack.
- Tighten the addr2line child's fd table (O_CLOEXEC /dev/null, close unused
pipe ends) so it inherits no stray descriptors.
Tests:
- BATS cases covering the structured backtrace, addr2line-resolved output, and
the addr2line-unavailable guidance path.
- arch_test.sh check_backtrace plus a "from signal context" assertion on
the crash test, run per architecture in CI under QEMU.
Signed-off-by: DL6ER <dl6er@dl6er.de>
getMACVendor() truncated the address to the 24-bit OUI (XX:YY:ZZ) and matched
that alone. Many OUIs are sub-divided into smaller MA-M (/28) and MA-S / IAB
(/36) assignments whose 24-bit prefix resolves only to "IEEE Registration
Authority" (or to no row at all) - so those devices showed a blank/useless
hardware vendor.
The macvendor table already stores these finer assignments in Wireshark manuf
form (e.g. "34:E1:D1:80/28", "00:1B:C5:00:00/36"), so no database change is
needed: reconstruct the candidate /24, /28 and /36 keys from the queried MAC in
SQL and let the longest match win. Existing /24 keys are matched unchanged, so
this is purely additive - no generator change, no DB rebuild, no transition.
Add a BATS test seeding MA-L/MA-M/MA-S rows that asserts each resolves, that two
devices under the same 24-bit parent resolve independently, and that an unknown
OUI yields no match.
Signed-off-by: RamSet <ramset@gmail.com>
The term "RSA PRIVATE KEY" has been replaced with the more general "PRIVATE KEY" as ECC keys are also accepted.
Please note that keys based on "Curve25519" and "Curve 448" (Ed25519/Ed448) for SSL/TLS server certificates are not yet supported by browsers, so they would cause an error during navigation if used.
According to the [Baseline Requirements for TLS Server Certificates](https://cabforum.org/working-groups/server/baseline-requirements/documents/), specifically section 6.1.5, currently only RSA keys greater than 2048 bits and ECDSA keys (NIST P-256, NIST P-384, or NIST P-521) are accepted.
Signed-off-by: DoctorD90 <DoctorD90@users.noreply.github.com>
The PID file path was previously user-configurable via files.pid in
pihole.toml. Service hook scripts executed as root read this value
without validation and used it in privileged file operations, enabling
local privilege escalation by a pihole user with direct write access
to pihole.toml.
Remove files.pid from the config system entirely and replace all
usages with the compile-time constant FTL_PID_FILE ("/run/pihole-FTL.pid")
defined in config.h. The PID file path has no good reason to be
user-configurable.
See: GHSA-6w8x-p785-6pm4
Signed-off-by: yubiuser <github@yubiuser.dev>
bcambl
DominikandGitHub
Henry SowellandGitHub
DL6ER
yubiuser
rdevshp
RamSet
DoctorD90
Adam WarnerandGitHub
