Skip to content

Replace assert() with proper error handling for user-triggerable conditions#2535

Open
sirus20x6 wants to merge 1 commit intosamtools:developfrom
sirus20x6:fix/assert-to-error
Open

Replace assert() with proper error handling for user-triggerable conditions#2535
sirus20x6 wants to merge 1 commit intosamtools:developfrom
sirus20x6:fix/assert-to-error

Conversation

@sirus20x6
Copy link

Summary

Several assert() calls guard conditions that can be triggered by user input. In release builds with NDEBUG, these are compiled out, causing silent undefined behavior or buffer overflows.

  • consensus.c: Replace 3 asserts (variant overlap, fasta buffer) with error() calls
  • convert.c: Replace 2 asserts (GT type check) with if-checks that warn and skip; add 2 allele index bounds checks before line->d.allele[ial] access
  • vcfconcat.c: Replace assert on bgzf block size with error() call
  • filter.c: Add NULL check on getenv("HOME") before passing to kputs

Test plan

  • Existing test suite passes (1920/1920)
  • Verify consensus/convert handle malformed input gracefully instead of crashing

…itions

assert() is compiled out in release builds (NDEBUG), causing silent
undefined behavior or buffer overflows when triggered by malformed input.

- consensus.c: Replace asserts on variant overlap trimming, unreachable
  overlap case, and fasta buffer bounds with error() calls
- convert.c: Replace GT type asserts with warning+skip; add allele index
  bounds checks in process_tgt and process_iupac_gt
- vcfconcat.c: Replace bgzf block size assert with error() call
- filter.c: Add NULL check for getenv("HOME") and getenv("HOMEDRIVE")/
  getenv("HOMEPATH") in expand_path()
@sirus20x6 sirus20x6 force-pushed the fix/assert-to-error branch from 929c0d9 to 7178ca4 Compare March 26, 2026 00:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant