Validate par and table inputs; add fuzz harnesses and SECURITY.md

[zzcgumn]

Summary

Closes five memory-safety defects in the input-validation and error-reporting
paths, and adds the fuzzing that found four of them.

The starting point was an audit question about whether DDS's input handling
warranted more attention. It did: SolveBoard() validates its input
thoroughly, but the par and CalcDDtable* entry points did not, and two of the
crashes turned out to be inside the validation and error-reporting logic
itself. Every fix is a range check; none touches the search algorithms.

Fixes

• Par table. SidesParBin() never checked res_table, so an out-of-range
  table overflowed char temp[8] in Par() (ASan: stack-buffer-overflow) and
  silently wrote 26 characters into a char[10] via SidesPar() — both while
  returning RETURN_NO_FAULT. Adds par_table_checks() and
  RETURN_PAR_TABLE_FAULT (-401), mirrored in DdsStatus.java and
  ErrorMessage().
• DealerPar() parameters. Neither vulnerable (indexes VUL_LOOKUP[4][2])
  nor dealer was validated; a negative dealer reached
  NUMBER_TO_PLAYER[static_cast<unsigned>(pno)], where the cast turns -1 into
  4294967295. Both are range-checked, and the unsigned casts are replaced with
  guarded helpers.
• CalcDDtable* card counts. No check that the four hands held equal
  numbers of cards, so a 51-card deal reached the search and read 14248 bytes
  past rel_rank_storage. Reachable from an ordinary PBN file short of a card —
  the reproducer contains only legal PBN characters. Adds
  table_deal_checks(), enforcing exactly the rules SolveBoard() already
  applied, so nothing is rejected that the solver would have accepted.
• DumpInput(). Indexed card_suit[5], card_hand[4] and card_rank[16]
  with the very values board_range_checks() was rejecting as out of range.
  Present in release builds, since it is compiled in unless
  DDS_NO_DUMP_ON_ERROR is defined.
• board_value_checks(). Validates currentTrickSuit[k] only when the
  matching rank is non-zero, but hand_rel_first derives from the card count
  rather than the trick entries, so a five-card deal with zero ranks indexed
  remainCards with an unchecked suit.

Fuzzing

Sanitizers already ran in CI, but nothing generated inputs for them. Adds
libFuzzer harnesses for convert_from_pbn(), CalcDDtablePBN(),
SolveBoard() and the par entry points. Each is also an ordinary cc_test
that replays a checked-in corpus, so it needs no libFuzzer and runs under
--config=asan/ubsan in normal CI — that is what keeps these fixed. The new
--config=fuzz uses the hermetic LLVM toolchain, since Apple's ships no
libFuzzer.

SECURITY.md

States the input trust model explicitly: DDS is an in-process library that
assumes trusted, well-formed input, with per-entry-point detail on what is and
is not validated, guidance for anyone exposing it to untrusted data, and
reporting via GitHub private advisories.

Notes for reviewers

• No ABI change. RETURN_PAR_TABLE_FAULT is additive; struct layouts are
  untouched.
• The CalcDDtable* validation is deliberately no stricter than
  SolveBoard()'s. It rejects unbalanced, duplicate-card and out-of-range-bit
  deals — inputs that previously produced a crash or meaningless table.
• One open hardening item is left deliberately: convert_from_pbn() still
  silently skips unrecognised characters, so an invalid rank now yields
  RETURN_CARD_COUNT rather than RETURN_PBN_FAULT. Tightening it risks
  rejecting PBN files with trailing newlines, so it is documented rather than
  changed. See library/tests/fuzz/findings/README.md.
• SECURITY.md points reporters at GitHub private advisories because the repo
  had no security contact; maintainers may prefer a specific address.

Test plan

• bazel test //library/... — 55/55
• bazel test --config=asan //library/... — 55/55
• bazel test --config=ubsan //library/... — 55/55
• bazel test //python/... (19) and //jni/... (5)
• Each fix verified by reverting it with the tests in place: the suite
  reproduces the original ASan report, and passes with the fix restored
• Post-fix campaigns: ~1.35M executions across the four harnesses
  (par 500k, pbn 500k, solve_board 300k, calc_dd_table_pbn 52k/240s) with
  no crashes
• CI on Linux and Windows/MSVC (only macOS was available locally)

🤖 Generated with Claude Code

LLVM Fuzz Tooling

[Copilot]

Pull request overview

This pull request hardens DDS input validation and error reporting, adds fuzzing and regression coverage, and documents the security model.

Changes:

• Validates par, dealer, vulnerability, and DD-table inputs.
• Adds fuzz harnesses, corpus replay tests, seeds, and Bazel configuration.
• Adds SECURITY.md and mirrors the new status code in Java.

Reviewed changes

Copilot reviewed 49 out of 69 changed files in this pull request and generated 6 comments.

Show a summary per file

File	Review summary
SECURITY.md	Nit (3 votes): Correct the claim that DDS has no persistent state; document process-local solver resources.
library/tests/par_validation_test.cpp	Adds par-validation regression coverage.
library/tests/fuzz/solve_board_fuzz.cpp	Adds solver fuzzing.
library/tests/fuzz/README.md	Documents fuzz workflows.
library/tests/fuzz/pbn_fuzz.cpp	Adds PBN fuzzing.
library/tests/fuzz/par_fuzz.cpp	Adds par API fuzzing.
library/tests/fuzz/fuzz.bzl	Defines reusable fuzz rules.
library/tests/fuzz/fuzz_corpus_main.cpp	Replays fuzz corpora.
library/tests/fuzz/findings/README.md	Documents fuzz findings.
library/tests/fuzz/corpus/pbn/west_first.txt	PBN regression seed.
library/tests/fuzz/corpus/pbn/void_suits.txt	PBN regression seed.
library/tests/fuzz/corpus/pbn/truncated.txt	Truncated-input seed.
library/tests/fuzz/corpus/pbn/south_first.txt	PBN regression seed.
library/tests/fuzz/corpus/pbn/north_first.txt	PBN regression seed.
library/tests/fuzz/corpus/pbn/no_colon.txt	PBN regression seed.
library/tests/fuzz/corpus/pbn/lowercase.txt	PBN regression seed.
library/tests/fuzz/corpus/pbn/extra_dots.txt	PBN regression seed.
library/tests/fuzz/corpus/pbn/empty.txt	Empty-input seed.
library/tests/fuzz/corpus/pbn/east_first.txt	PBN regression seed.
library/tests/fuzz/corpus/pbn/bad_rank_parser_only.txt	Parser regression seed.
library/tests/fuzz/corpus/pbn/bad_compass.txt	PBN regression seed.
library/tests/fuzz/corpus/par/negative.bin	Invalid-table seed.
library/tests/fuzz/corpus/calc_dd_table_pbn/west_first.txt	PBN table-calculation seed.
library/tests/fuzz/corpus/calc_dd_table_pbn/void_suits.txt	PBN table-calculation seed.
library/tests/fuzz/corpus/calc_dd_table_pbn/unbalanced_51_cards.txt	Unbalanced-deal regression seed.
library/tests/fuzz/corpus/calc_dd_table_pbn/truncated.txt	Truncated-deal seed.
library/tests/fuzz/corpus/calc_dd_table_pbn/south_first.txt	PBN table-calculation seed.
library/tests/fuzz/corpus/calc_dd_table_pbn/north_first.txt	PBN table-calculation seed.
library/tests/fuzz/corpus/calc_dd_table_pbn/no_colon.txt	PBN table-calculation seed.
library/tests/fuzz/corpus/calc_dd_table_pbn/lowercase.txt	PBN table-calculation seed.
library/tests/fuzz/corpus/calc_dd_table_pbn/extra_dots.txt	PBN table-calculation seed.
library/tests/fuzz/corpus/calc_dd_table_pbn/empty.txt	Empty-input seed.
library/tests/fuzz/corpus/calc_dd_table_pbn/east_first.txt	PBN table-calculation seed.
library/tests/fuzz/corpus/calc_dd_table_pbn/bad_rank.txt	PBN table-calculation seed.
library/tests/fuzz/corpus/calc_dd_table_pbn/bad_compass.txt	PBN table-calculation seed.
library/tests/fuzz/calc_dd_table_pbn_fuzz.cpp	Adds PBN table-calculation fuzzing.
library/tests/fuzz/BUILD.bazel	Defines fuzz and replay targets.
library/tests/deal_input_validation_test.cpp	Adds deal and diagnostic validation tests.
library/tests/BUILD.bazel	Registers regression tests.
library/src/table_deal_validate.hpp	Critical (3 votes): Apply validation to public C++ and shim table-calculation paths. Critical (2 votes): Reject excessive or negative batch counts before fixed-array conversion.
library/src/solver_if.cpp	Hardens trick-suit validation.
library/src/par.cpp	Moderate (3 votes): Validate vulnerable in direct Par() and SidesPar() paths.
library/src/par_validate.hpp	Adds par-validation helpers.
library/src/init.cpp	Maps the new status code.
library/src/dump.cpp	Hardens diagnostic formatting.
library/src/dealer_par.cpp	Validates dealer and vulnerability parameters.
library/src/calc_tables.cpp	Critical (2 votes): Protect table-count boundaries before multiplication or conversion and add boundary coverage.
library/src/BUILD.bazel	Exposes validation sources and dependencies.
library/src/api/dll.h	Nit (3 votes): Add RETURN_PAR_TABLE_FAULT to doc/dll-description.md.
jni/java/org/dds/ffm/DdsStatus.java	Mirrors the new status code.
.bazelrc	Adds fuzzing configuration.

Suppressed comments (6)

SECURITY.md:67

• The library intentionally accepts partial deals when all four hands have equal card counts; the existing partial-deal tests exercise one-card-per-hand inputs. Requiring exactly 13 cards here would cause callers to reject valid partial deals, so qualify 13 cards as the full-deal case and otherwise require equal counts.

1. **Validate at your boundary.** Reject deals that are not 13 cards per hand
   and tables whose entries fall outside 0-13, before calling DDS.

SECURITY.md:40

• Only DealerPar()/DealerParBin() range-check dealer and vulnerable. In the default build, Par(), SidesPar() and SidesParBin() do not range-check vulnerable; out-of-range values are treated as non-vulnerable and can return a successful but incorrect result. Either add the shared parameter check to every affected entry point or narrow this policy statement.

- The par entry points validate the double dummy table
  (`par_table_checks()`) and their `dealer` and `vulnerable` parameters. Both
  checks were added after fuzzing found an out-of-range table overflowing a
  fixed character buffer and a negative `dealer` indexing a string table.

jni/java/org/dds/ffm/DdsStatus.java:80

• This new status is mirrored in Java, but the public .NET wrappers ParSide, ParDealer, and DealerParBothSides cast results to SolveBoardResult; that enum and GetRcErrorMessage() have no -401. A C# caller therefore receives the new table-fault result as an unnamed status and gets Unknown error from the wrapper. Add the status to the .NET binding as well, or explicitly document that it is not exposed there.

    /** Double dummy table entry outside the range 0 to 13. */
    public static final int RETURN_PAR_TABLE_FAULT = -401;

library/src/calc_tables.cpp:552

• This is a separate board-expansion path and is not covered by the malformed-deal regression, which only exercises legacy CalcAllTables(). Without an CalcAllTablesX() regression using an unbalanced deal, a future removal or relocation of this loop would let the X API reach the solver with the same invalid input unchecked; add a direct X-path test (and the corresponding PBNX case if that path is intended to be covered).

    for (int m = 0; m < numDeals; m++)
    {
      int const check = table_deal_checks(deals[m]);
      if (check != RETURN_NO_FAULT)
        return check;

library/src/calc_tables.cpp:552

• This validation does not protect the subsequent batch-size arithmetic: nboards = numDeals * included is computed as a signed int. For positive inputs where that product wraps to a small positive value, the vector is undersized but the expansion loop still increments ind for the full mathematical product and writes past boards; other large inputs invoke undefined signed overflow or unbounded allocation. Compute the product in a checked size_t (and use a matching index type), or reject unrepresentable requests before allocating.

    for (int m = 0; m < numDeals; m++)
    {
      int const check = table_deal_checks(deals[m]);
      if (check != RETURN_NO_FAULT)
        return check;

library/tests/fuzz/solve_board_fuzz.cpp:44

• The selector mapping never reaches the upper-bound validation cases for solutions or mode: their ranges are -1..3 and -1..2, respectively, so solutions > 3 and mode > 2 cannot be fuzzed through this harness. Map these fields to ranges that include 4 and 3 (or consume additional selector bytes) so both sides of each documented range are exercised.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

Pull request overview

Copilot reviewed 53 out of 90 changed files in this pull request and generated 7 comments.

Suppressed comments (9)

SECURITY.md:79

• This guidance says to reject every deal that is not 13 cards per hand, but the newly documented table_deal_checks() and the table APIs intentionally accept equal-card-count partial deals; the regression suite even uses one card per hand. Please distinguish an application's complete-deal policy from DDS's actual accepted input, or callers may incorrectly reject valid partial-deal use.

1. **Validate at your boundary.** Reject deals that are not 13 cards per hand
   and tables whose entries fall outside 0-13, before calling DDS.

SECURITY.md:91

• This guidance overstates the side effect: DumpInput() is called by SolveBoard()'s validation paths, but the newly added par/table validation paths return errors without calling it (and CalcDDtable* does not invoke it either). Say it writes dump.txt when a SolveBoard() rejection reaches DumpInput(), so callers do not expect an unexpected file for every validation error.

5. Note that `DumpInput()` writes a `dump.txt` file into the process working
   directory whenever input is rejected. Define `DDS_NO_DUMP_ON_ERROR` to
   compile it out.

library/src/api/dll.h:156

• RETURN_PAR_TABLE_FAULT is now returned for invalid DdTableResults, but the Python error mapper in python/src/bindings.cpp:33-50 does not classify it as an input error. Consequently par(), calc_par_from_table(), and dealer_par() raise RuntimeError for bad table values even though their docstrings promise ValueError. Add this code to the py::value_error cases and cover it in the Python tests.

#define RETURN_PAR_TABLE_FAULT -401

library/src/api/dll.h:157

• par_table_checks() deliberately returns RETURN_PAR_TABLE_FAULT for a null table as well as for an out-of-range entry. This text is therefore misleading for the documented null-input path; broaden the message (and its mirrored documentation) to mention a null/invalid table, or use a status whose text describes the null argument.

#define TEXT_PAR_TABLE_FAULT "Double dummy table entry outside the range 0 to 13"

library/src/calc_tables.cpp:582

• CalcAllTablesX() walks and validates every deals[m] before applying the numDeals * included overflow guard below. A count above the guard therefore still causes an O(numDeals) scan (and caller-array dereferences) before the intended RETURN_TOO_MANY_TABLES, defeating the early resource check for large or untrusted counts. Move this product check to immediately after included is known and before this loop.

    for (int m = 0; m < numDeals; m++)
    {
      int const check = table_deal_checks(deals[m]);
      if (check != RETURN_NO_FAULT)
        return check;
    }

library/src/dump.cpp:285

• suit_text() and rank_text() are used to report current-trick values that have stricter semantic domains than the backing arrays: DDS_STRAINS admits no-trump (4), and card_rank[0/1/15] are sentinel characters. A rejected current-trick suit 4 is therefore logged as N, and invalid ranks can be logged as x/-, rather than the offending values. Use context-specific bounds or raw-value fallbacks for current-trick diagnostics.

auto suit_text(const int suit) -> std::string
{
  if (suit < 0 || suit >= DDS_STRAINS)
    return "?(" + std::to_string(suit) + ")";
  return std::string(1, static_cast<char>(card_suit[suit]));

library/src/dump.cpp:300

• card_rank[0], [1], and [15] are sentinel entries, while board_range_checks() permits only a nonzero trick rank in 2..14. Therefore invalid ranks such as 1 or 15 are reported as x or - rather than the raw invalid value, despite this helper's diagnostic contract. Check the semantic 2..14 range here (or pass it explicitly).

auto rank_text(const int rank) -> std::string
{
  constexpr int card_rank_size = 16;
  if (rank < 0 || rank >= card_rank_size)
    return "?(" + std::to_string(rank) + ")";
  return std::string(1, static_cast<char>(card_rank[rank]));

library/src/par_validate.hpp:38

• This new status is an input-validation error, but python/src/bindings.cpp::throw_on_dds_error() does not classify RETURN_PAR_TABLE_FAULT as ValueError. As a result, calc_par_from_table() now raises RuntimeError for an out-of-range res_table, despite its documented ValueError contract for invalid table input. Add the new status to the binding's validation cases and cover it with a Python regression test.

  for (int d = 0; d < DDS_STRAINS; d++)
    for (int h = 0; h < DDS_HANDS; h++)
      if (tablep->res_table[d][h] < 0 || tablep->res_table[d][h] > 13)
        return RETURN_PAR_TABLE_FAULT;

library/tests/fuzz/solve_board_fuzz.cpp:25

• The argument to SetMaxThreads() is ignored in the current implementation, so this comment incorrectly says the call selects one worker. The call is still useful here to initialize static solver memory, but the comment should describe that behavior rather than a thread cap.

[tameware]

I'll have a look once all comments are resolved and we have a clean Copilot review. I still have Copilot credits - let me know if you'd like a run.

[Copilot]

Pull request overview

Copilot reviewed 53 out of 90 changed files in this pull request and generated no new comments.

Suppressed comments (5)

Previously missed (1) — in code that hasn't changed since the last review.

library/src/api/dll.h:156

• RETURN_PAR_TABLE_FAULT is now the native error returned for invalid par/calc_par_from_table tables, but throw_on_dds_error() does not include it in its input-validation cases. As a result, the Python bindings raise RuntimeError for exactly the invalid-table inputs whose docstrings promise ValueError (and calc_par can likewise surface this through its par stage). Add this status to the Python py::value_error mapping and cover the binding behavior with a regression test.

#define RETURN_PAR_TABLE_FAULT -401

SECURITY.md:29

• This new guidance correctly says SetMaxThreads() is ignored, but the repository's public legacy documentation still contradicts it: doc/dll-description.md and docs/dotnet_interface.md describe SetMaxThreads as limiting the worker count. Since this section is specifically intended to prevent unsafe resource assumptions, please update those references (and any generated variants) in the same change so users do not receive conflicting limits guidance.

- `SetMaxThreads()` sets nothing. It is a deprecated alias of
  `InitializeStaticMemory()` and its argument is ignored; internal batch
  threading was removed. Do not treat it as a resource limit. In the modern
  C++ API the embedding application controls concurrency, typically with one
  `SolverContext` per worker thread.

library/src/api/dll.h:157

• par_table_checks() deliberately returns this code for a null table as well as for an out-of-range entry, but ErrorMessage() then reports only the entry-range case. A caller passing nullptr therefore receives a misleading diagnostic; make the new text cover both invalid-table cases (and keep the corresponding API error tables in sync).

#define TEXT_PAR_TABLE_FAULT "Double dummy table entry outside the range 0 to 13"

library/src/calc_tables.cpp:581

• Please move the deal×strain overflow preflight ahead of this loop. For any numDeals > INT_MAX / included, CalcAllTablesX() is guaranteed to return RETURN_TOO_MANY_TABLES, but it currently performs an O(numDeals) validation scan first, making a count that should be rejected a potentially very long operation and defeating the cheap resource guard. Place this check immediately after the included/par checks (and share the preflight with the PBN variant).

    for (int m = 0; m < numDeals; m++)
    {
      int const check = table_deal_checks(deals[m]);
      if (check != RETURN_NO_FAULT)
        return check;

library/src/dump.cpp:318

• rank_text() is only used for currentTrickRank, whose valid range is 2..14 in board_range_checks(). Accepting 0..15 here renders rejected ranks 1 and 15 as the sentinel characters x and - instead of the raw ?(...) fallback promised above, hiding the invalid value in dump.txt; use the actual trick-rank range for this helper.

auto rank_text(const int rank) -> std::string
{
  constexpr int card_rank_size = 16;
  if (rank < 0 || rank >= card_rank_size)
    return "?(" + std::to_string(rank) + ")";
  return std::string(1, static_cast<char>(card_rank[rank]));

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[zzcgumn]

@tameware , could you give this another Copilot review please?

[tameware]

In progress.

…

On Tue, Aug 25, 2026 at 2:54 PM Martin Nygren ***@***.***> wrote: *zzcgumn* left a comment (dds-bridge/dds#349) <#349 (comment)> @tameware <https://github.com/tameware> , could you give this another Copilot review please? — Reply to this email directly, view it on GitHub <#349?email_source=notifications&email_token=ABC4PYAMIYUTRQAKDISBG4D5LWD73A5CNFSNUABFM5UWIORPF5TWS5BNNB2WEL2JONZXKZKDN5WW2ZLOOQXTKNBRGA3DMNBZGI22M4TFMFZW63VHNVSW45DJN5XKKZLWMVXHJLDGN5XXIZLSL5RWY2LDNM#issuecomment-5410664925>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ABC4PYAVLZIA7NOO5CPDZB35LWD73AVCNFSNUABEKJSXA33TNF2G64TZHMZDMOJYHAZDANR3JFZXG5LFHM2TEMZUGY3TCOBUGCQXMAQ> . You are receiving this because you were mentioned.Message ID: ***@***.***>