Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix issue created in PR#1848 #1849

Merged
merged 1 commit into from
Oct 16, 2024
Merged

Conversation

jkbonfield
Copy link
Contributor

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

Note the following test triggered it, but it's only for that most recent commit and 1.21 was fine so this is a brief regression.

@SQ	SN:foo	LN:999
x	4	*	1	0	*	*	0	0	ACGT	PQRS

Copy link
Member

@daviesrob daviesrob left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've managed to come up with a better input to reproduce the original problem:

@SQ	SN:1	LN:100
I	4	*	1	0	*	*	0	0	CCTAGCCCTAACCCTAACCCTAACCCTAGCCTAAGCCTAAGCCTAAGCCTAAGCCTAAGCCTAAGCCTAAGCCTAAGCCTAAGCCTAAGCCTAAGCCTAA	#############################@B?8B?BA@@DDBCDDCBC@CDCDCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCC
II	0	1	1	1	100M	*	0	0	CCTAGCCCTAACCCTAACCCTAACCCTAGCCTAAGCCTAAGCCTAAGCCTAAGCCTAAGCCTAAGCCTAAGCCTAAGCCTAAGCCTAAGCCTAAGCCTAA	#############################@B?8B?BA@@DDBCDDCBC@CDCDCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCC

As it makes no claim as to version or sort order, this may even be a valid SAM file.

cram/cram_encode.c Outdated Show resolved Hide resolved
cram/cram_encode.c Outdated Show resolved Hide resolved
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
@daviesrob daviesrob merged commit 4c1acb8 into samtools:develop Oct 16, 2024
9 checks passed
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.

2 participants