Fix @PG linking when records make a loop
#1702
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
If all the @pg records passed to sam_hdr_link_pg() form a single PP loop, all entries in the sam_hrecs_t::pg_end array it builds get set to -1, indicating that there are no chain start points. These entries are then removed to make the final list, but due to a bug in handling the case where there are no PP links the number of entries was incorrectly set to 1 instead of 0. This could lead to an out-of-bounds read in sam_hdr_add_pg() when linking new @pg entries to the existing ones. Fix by ensuring that only valid end points are returned in the sam_hrecs_t::pg_end array, and the length is set to zero if no ends are detected due to a loop. Adds a warning if a @pg record is found with a PP link to itself. Detecting longer loops is left for future work. Fixes another warning which incorrectly said 'SN' instead of 'ID' in its message. Adds an assert() in sam_hdr_add_pg() to catch any other cases of the out-of-bounds read. Adds tests for loopy @pg records. Thanks to Octavio Galland for the bug report.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
If all the
@PGrecords passed tosam_hdr_link_pg()form a singlePPloop, all entries in thesam_hrecs_t::pg_endarray it builds get set to -1, indicating that there are no chain start points. These entries are then removed to make the final list, but due to a bug in handling the case where there are noPPlinks the number of entries was incorrectly set to 1 instead of 0. This could lead to an out-of-bounds read insam_hdr_add_pg()when linking new@PGentries to the existing ones.Fix by ensuring that only valid end points are returned in the
sam_hrecs_t::pg_end array, and the length is set to zero if no ends are detected due to a loop.Adds a warning if a
@PGrecord is found with a PP link to itself. Detecting longer loops is left for future work. Fixes another warning which incorrectly said 'SN' instead of 'ID' in its message.Adds an
assert()insam_hdr_add_pg()to catch any other cases of the out-of-bounds read.Adds tests for loopy
@PGrecords.Thanks to @OctavioGalland for the bug report. Fixes #1694