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

Update NEWS entry: CVE-2020-36403 affected all older versions of HTSlib #1447

Merged
merged 1 commit into from Jun 9, 2022

Conversation

jmarshall
Copy link
Member

@jmarshall jmarshall commented Jun 9, 2022

PR #1311 trusted the analysis in google/oss-fuzz-vulns's vulns/htslib/OSV-2020-955.yaml as to what HTSlib versions were affected by CVE-2020-36403:

    - introduced: dd6f0b72c92591252bb77818663629cc1a129949
    - fixed: dcd4b7304941a8832fba2d0fc4c1e716e7a4e72c
  versions:
  - '1.10'
  - 1.10.1
  - 1.10.2

It is accurate that the issue was fully fixed in dcd4b73, but the identified “introduced” commit dd6f0b7 is spurious and unrelated.

It turns out that the fuzz testing's test data file also has an invalid #CHROM line which resulted in an error message in HTSlib versions ≤1.9:

$ bcftools-1.9 view -o /dev/null clusterfuzz-OSV-2020-955.vcf
[E::bcf_hdr_add_sample] Empty sample name: trailing spaces/tabs in the header line?
Abort trap: 6

$ bcftools-1.10 view -o /dev/null clusterfuzz-OSV-2020-955.vcf
[…]
Segmentation fault: 11

$ bcftools-1.10.2 view -o /dev/null clusterfuzz-OSV-2020-955.vcf
[…]
Segmentation fault: 11

$ bcftools-1.11 view -o /dev/null clusterfuzz-OSV-2020-955.vcf
[…]
[E::vcf_parse_format] Excessive memory required by FORMAT fields at @SQ:0
Error: VCF parse error

$ bcftools-1.14 view -o /dev/null clusterfuzz-OSV-2020-955.vcf
[E::bcf_hdr_parse_sample_line] Could not parse the "#CHROM.." line, either the fields are incorrect or spaces are present instead of tabs: […]
Failed to read from clusterfuzz-OSV-2020-955.vcf: could not parse header

This error message and early exit masked the segfault caused by the actual issue, namely a VCF record whose in-memory representation requires more than 2GiB. Hence the fuzzing harness incorrectly concluded that versions ≤1.9 were unaffected by the issue.

The attached gen-huge-vcf.pl script generates an otherwise clean VCF file that exhibits the “record needs more than 2 GiB in memory” problem. This segfaults all the way back to HTSlib 1.0:

$ bcftools-1.0 view -H huge.vcf
Segmentation fault: 11

$ bcftools-1.3.1 view -H huge.vcf
Segmentation fault: 11

$ bcftools-1.9 view -H huge.vcf
Segmentation fault: 11

$ bcftools-1.10 view -H huge.vcf
Segmentation fault: 11

$ bcftools-1.10.2 view -H huge.vcf
Segmentation fault: 11

$ bcftools-1.11 view -H huge.vcf
[E::vcf_parse_format] Excessive memory required by FORMAT fields at chr1:100
Error: VCF parse error

$ bcftools-1.14 view -H huge.vcf
[E::vcf_parse_format] Excessive memory required by FORMAT fields at chr1:100
Error: VCF parse error

Thus all versions of HTSlib prior to the fixes in 1.11 were susceptible to this issue. This is unsurprising as, prior to the check added in PR #1044, the ks_resize() code was unchanged since the beginning.

This issue was fixed in 1.11 by PRs samtools#1044 and samtools#1104. It was detected via
fuzz testing (https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=24097)
but the Reproducer Testcase also has an invalid `#CHROM` line which
resulted in an error message in HTSlib versions <= 1.9.

This error message masked the segfault caused by the actual issue, namely
a VCF record whose in-memory representation requires more than 2GiB.
A clean test case produces a segfault all the way back to HTSlib 1.0.
@jkbonfield
Copy link
Contributor

Nice research and thanks.

@jkbonfield jkbonfield merged commit fee3bbb into samtools:develop Jun 9, 2022
8 checks passed
@jmarshall jmarshall deleted the cve-affected branch June 9, 2022 13:42
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.

None yet

2 participants