ConformalLabpp/doc/reviewer/input-validation-audit-2026-05-31.md

Input-Validation & Malformed-Input Audit — ConformalLabpp

Date: 2026-05-31 Auditor: External reviewer (Claude Opus 4.8) Scope: Robustness of the library's input boundaries — mesh I/O (OFF/OBJ/PLY) and result (de)serialization (JSON/XML) — against malformed, adversarial, or out-of-domain input. Focus: What happens with bad input. Distinct from the error-handling audit, which covered internal error propagation and the deliberate throw paths.

Status legend: 🔴 Critical · 🟡 Important · 🔵 Polish

✅ Resolution status (2026-05-31, Session 1): V1/V2/V4 ✅ (JSON parse + field checks, XML stoi/stod guarded — all surface as std::runtime_error with path), V3 ✅ (NaN/Inf vertex-coordinate guard in load_mesh). V5 (XML strict-reject) and V6 (DOF-size check) open — scheduled S3 in finding-orchestration.md. 298/298 tests green.

Threat model: this is a scientific library, not a network service, so the bar is "fail cleanly and diagnosably", not "resist attackers". But meshes and result files routinely come from other tools, other people, and older versions of this code — so malformed input is a when, not an if.

---

Summary table

ID	Sev	Title	Location
V1	🟡	JSON deserialization has no error handling — nlohmann exceptions leak the abstraction, not the library's runtime_error	serialization.hpp:96-110
V2	🟡	XML parser uses unchecked std::stoi/std::stod on attributes → uncaught std::invalid_argument on garbage	serialization.hpp:258-261
V3	🟡	No validation of vertex coordinate values on load — NaN/Inf coordinates flow silently into the solver	mesh_io.hpp:56-66
V4	🟡	JSON loader accesses nested keys (j["solver"]["converged"]) without contains checks → throws type_error/creates nulls on schema drift	serialization.hpp:104-108
V5	🔵	Hand-rolled XML parser assumes one element per line and no escaping — silently mis-reads valid-but-reformatted XML	serialization.hpp:248-270
V6	🔵	No upper bound / sanity check on DOF-vector length vs mesh size when loading	serialization.hpp

---

V1 — 🟡 JSON deserialization leaks nlohmann exceptions

Evidence

serialization.hpp:96-108:

std::ifstream ifs(path);
if (!ifs) throw std::runtime_error("Cannot open: " + path);
json j; ifs >> j;                       // ← malformed JSON → nlohmann::parse_error
...
res->converged = j["solver"]["converged"].get<bool>();   // ← wrong type → type_error

Problem

The "file missing" case is handled with the library's own std::runtime_error (consistent with mesh_io.hpp). But the moment the file exists and is malformed, ifs >> j throws nlohmann::json::parse_error and the typed accessors throw nlohmann::json::type_error — neither is the library's documented error type, and neither carries the file path. A caller writing catch (const std::runtime_error&) (the pattern the examples use) will miss these. (nlohmann exceptions derive from std::exception, not std::runtime_error.)

Fix

Wrap the parse + extraction in try { ... } catch (const json::exception& e) and rethrow as std::runtime_error("conformallab: malformed JSON in " + path + ": " + e.what()), matching the mesh_io.hpp boundary convention.

Acceptance criteria

• Loading a truncated/garbage JSON throws std::runtime_error with the path.
• A negative test (EXPECT_THROW(..., std::runtime_error)) covers it.

---

V2 — 🟡 XML parser: unchecked stoi/stod on attributes

Evidence

serialization.hpp:258-261:

res->iterations    = std::stoi(detail_xml::xml_get_attr(line, "iterations"));
res->grad_inf_norm = std::stod(detail_xml::xml_get_attr(line, "grad_inf_norm"));

Problem

If the attribute is missing or non-numeric (xml_get_attr returns "" or garbage), std::stoi("") throws std::invalid_argument and out-of-range values throw std::out_of_range — both uncaught, both not the library's runtime_error, neither carrying the path. Same abstraction leak as V1, different exception family.

Fix

Parse defensively: check the attribute is non-empty and numeric, or wrap in try/catch and rethrow as std::runtime_error with context (path + attribute name).

Acceptance criteria

• An XML file with a non-numeric iterations attribute throws std::runtime_error, not std::invalid_argument.

---

V3 — 🟡 No validation of vertex coordinates on load

Evidence

mesh_io.hpp:56-66 — load_mesh checks read success and is_triangle_mesh, but does not inspect the vertex coordinate values.

Problem

A mesh file containing nan/inf coordinates (common from a crashed upstream tool, or 1e308-style overflow) reads in cleanly. The NaN then propagates: edge lengths → lambda0 → gradient → Hessian → the Newton solver, where it silently poisons the result (converged may even read true if NaN < tol evaluates falsely in a way that exits the loop). This is the worst kind of failure: silent, late, and hard to trace back to the input.

Fix

After read_mesh, scan vertex coordinates for finiteness (std::isfinite) and throw std::runtime_error("conformallab: non-finite vertex coordinate in " + filename) on the first offender. O(V), negligible cost at the I/O boundary — the same philosophy as the existing is_triangle_mesh guard.

Acceptance criteria

• Loading a mesh with a NaN/Inf coordinate throws std::runtime_error.
• Covered by a negative test.

---

V4 — 🟡 JSON loader assumes schema without checking

Evidence

serialization.hpp:104-108 reads j["solver"]["converged"], ["iterations"], ["grad_inf_norm"] and j.at("dof_vector") — only geometry is guarded with j.contains(...) (:99).

Problem

operator[] on a missing key in nlohmann inserts a null (for non-const json) or throws; .get<bool>() on null throws type_error. So a result file written by a future or older schema version fails with an opaque nlohmann error rather than a clear "missing field X" message. (dof_vector correctly uses .at(), which throws a clear out_of_range — but still not the library's type.)

Fix

Validate the expected keys up front (or use .at() uniformly + the V1 try/catch), producing a single std::runtime_error("conformallab: result JSON missing field 'solver.converged'")-style message.

Acceptance criteria

• A JSON missing solver.iterations yields a runtime_error naming the field.

---

V5 — 🔵 Hand-rolled XML parser is format-fragile

Evidence

serialization.hpp:248-270 parses line-by-line with line.find("<Solver") etc. and assumes each element (and the <DOFVector>…</DOFVector> text) fits patterns on one line.

Problem

This is not an XML parser — it is a line-pattern matcher. A semantically identical file that pretty-prints elements across multiple lines, or adds an XML declaration / namespace / attribute reordering, will be silently mis-read (fields default to zero/empty) rather than rejected. Round-trips written by this same code work; nothing else is guaranteed.

Fix

Either (a) document the XML format as a strict internal-only subset (and reject input that doesn't match, rather than mis-reading it), or (b) if interop matters, use a real XML parser. Given JSON is the primary format, (a) is the pragmatic choice — but the "silently mis-read" behavior must become "explicitly reject".

Acceptance criteria

• Reformatted-but-valid XML is either parsed correctly or rejected with an error — never silently mis-read into zeros.

---

V6 — 🔵 No DOF-vector vs mesh sanity check

Evidence

load_result_json returns x = j.at("dof_vector") with no check that x.size() matches the DOF count of the mesh it will be applied to.

Problem

A result file from a different mesh loads happily; the size mismatch only surfaces later (out-of-bounds or wrong-answer) when x is indexed against the new mesh.

Fix

Where the loaded x is paired with a mesh, assert/throw on a size mismatch with a clear message. (May belong at the call site rather than the loader.)

---

What is already good

• The file-missing path is handled correctly and consistently (runtime_error + path) in both mesh_io.hpp and serialization.hpp.
• load_mesh already enforces the triangle-mesh precondition at the boundary — the right place and the right philosophy; V3 just extends it to coordinate finiteness.
• dof_vector uses .at() (throwing) rather than operator[] — partial good practice that V4 asks to make uniform.
• JSON (nlohmann) is a robust, well-tested parser — the gap is only the missing try/catch around it (V1), not the parser itself.

Suggested order

1. V3 (NaN/Inf coordinate guard) — highest impact, prevents silent solver poisoning.
2. V1 + V4 (JSON error handling + schema checks) — do together.
3. V2 (XML stoi/stod) — same pattern as V1.
4. V5 + V6 (XML strictness, size check) — polish.