Skip to content

Commit

Permalink
Fix issue created in PR#1848
Browse files Browse the repository at this point in the history
We shouldn't be attempting to create embedded reference sequences for
CRAM containers with reads mapped to chr -1 (ie unmapped).  We don't
permit embed_ref in multi-ref mode and it's nonsensical for entirely
unmapped data.

The only real fix needed here is "c->ref_id < 0" just before
calling cram_generate_reference(), but checking elsewhere can sidestep
other potential issues and we have safety in checking in more than one
place.

Credit to OSS_Fuzz
Fixes oss-fuzz issue 372547397
  • Loading branch information
jkbonfield committed Oct 16, 2024
1 parent ca92061 commit 4c1acb8
Show file tree
Hide file tree
Showing 2 changed files with 10 additions and 2 deletions.
10 changes: 8 additions & 2 deletions cram/cram_encode.c
Original file line number Diff line number Diff line change
Expand Up @@ -1842,7 +1842,7 @@ int cram_encode_container(cram_fd *fd, cram_container *c) {
// Don't try embed ref if we repeatedly fail
pthread_mutex_lock(&fd->ref_lock);
int failed_embed = (fd->no_ref_counter >= 5); // maximum 5 tries
if (!failed_embed && c->embed_ref == -2) {
if (!failed_embed && c->embed_ref == -2 && c->ref_id >= 0) {
hts_log_warning("Retrying embed_ref=2 mode for #%d/5", fd->no_ref_counter);
fd->no_ref = c->no_ref = 0;
fd->embed_ref = c->embed_ref = 2;
Expand Down Expand Up @@ -1921,6 +1921,12 @@ int cram_encode_container(cram_fd *fd, cram_container *c) {
// Do not confuse with fd->ref_free which is a pointer to a
// reference string to free.
c->ref_free = 1;
} else {
// Double check for broken input. We shouldn't have
// embedded references enabled for unmapped data, but our
// data could be broken.
embed_ref = 0;
no_ref = c->no_ref = 1;
}
}
c->ref_seq_id = c->ref_id;
Expand Down Expand Up @@ -1967,7 +1973,7 @@ int cram_encode_container(cram_fd *fd, cram_container *c) {

// Embed consensus / MD-generated ref
if (embed_ref == 2) {
if (cram_generate_reference(c, s, r1) < 0) {
if (c->ref_id < 0 || cram_generate_reference(c, s, r1) < 0) {
// Should this be a permanent thing via fd->no_ref?
// Doing so means we cannot easily switch back again should
// things fix themselves later on. This is likely not a
Expand Down
2 changes: 2 additions & 0 deletions cram/cram_io.c
Original file line number Diff line number Diff line change
Expand Up @@ -4954,6 +4954,8 @@ int cram_write_SAM_hdr(cram_fd *fd, sam_hdr_t *hdr) {
hts_log_warning("NOTE: the CRAM file will be bigger "
"than using an external reference");
pthread_mutex_lock(&fd->ref_lock);
// Best guess. It may be unmapped data with broken
// headers, in which case this will get ignored.
fd->embed_ref = 2;
pthread_mutex_unlock(&fd->ref_lock);
break;
Expand Down

0 comments on commit 4c1acb8

Please sign in to comment.