Skip to content

Commit 0c0623f

Browse files
daviesrobjkbonfield
authored andcommitted
Add NULL checks to sam_hdr_read(), sam_hdr_write(); add checks to test/sam.c copy_check_alignment() (#730)
* Check for NULL fp in sam_hdr_read(), sam_hdr_write() * Add more tests for failure in copy_check_alignment() Catch problems like sam_open() and sam_read1() failing. Prevents a seg fault if sam_open() fails on either the input or output file. Fixes #728 (A SEGV signal occurred when running program sam in test directory).
1 parent 6565adf commit 0c0623f

2 files changed

Lines changed: 35 additions & 6 deletions

File tree

sam.c

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1007,6 +1007,11 @@ static bam_hdr_t *sam_hdr_sanitise(bam_hdr_t *h) {
10071007

10081008
bam_hdr_t *sam_hdr_read(htsFile *fp)
10091009
{
1010+
if (!fp) {
1011+
errno = EINVAL;
1012+
return NULL;
1013+
}
1014+
10101015
switch (fp->format.format) {
10111016
case bam:
10121017
return sam_hdr_sanitise(bam_hdr_read(fp->fp.bgzf));
@@ -1061,7 +1066,7 @@ bam_hdr_t *sam_hdr_read(htsFile *fp)
10611066

10621067
int sam_hdr_write(htsFile *fp, const bam_hdr_t *h)
10631068
{
1064-
if (!h) {
1069+
if (!fp || !h) {
10651070
errno = EINVAL;
10661071
return -1;
10671072
}

test/sam.c

Lines changed: 29 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -459,29 +459,53 @@ static void copy_check_alignment(const char *infname, const char *informat,
459459
samFile *in = sam_open(infname, "r");
460460
samFile *out = sam_open(outfname, outmode);
461461
bam1_t *aln = bam_init1();
462-
bam_hdr_t *header;
462+
bam_hdr_t *header = NULL;
463+
int res;
464+
465+
if (!in) {
466+
fail("couldn't open %s", infname);
467+
goto err;
468+
}
469+
if (!out) {
470+
fail("couldn't open %s with mode %s", outfname, outmode);
471+
goto err;
472+
}
473+
if (!aln) {
474+
fail("bam_init1() failed");
475+
goto err;
476+
}
463477

464478
if (outref) {
465-
if (hts_set_opt(out, CRAM_OPT_REFERENCE, outref) < 0)
479+
if (hts_set_opt(out, CRAM_OPT_REFERENCE, outref) < 0) {
466480
fail("setting reference %s for %s", outref, outfname);
481+
goto err;
482+
}
467483
}
468484

469485
header = sam_hdr_read(in);
486+
if (!header) {
487+
fail("reading header from %s", infname);
488+
goto err;
489+
}
470490
if (sam_hdr_write(out, header) < 0) fail("writing headers to %s", outfname);
471491

472-
while (sam_read1(in, header, aln) >= 0) {
492+
while ((res = sam_read1(in, header, aln)) >= 0) {
473493
int mod4 = ((intptr_t) bam_get_cigar(aln)) % 4;
474494
if (mod4 != 0)
475495
fail("%s CIGAR not 4-byte aligned; offset is 4k+%d for \"%s\"",
476496
informat, mod4, bam_get_qname(aln));
477497

478498
if (sam_write1(out, header, aln) < 0) fail("writing to %s", outfname);
479499
}
500+
if (res < -1) {
501+
fail("failed to read alignment from %s", infname);
502+
}
480503

504+
err:
481505
bam_destroy1(aln);
482506
bam_hdr_destroy(header);
483-
sam_close(in);
484-
sam_close(out);
507+
if (in) sam_close(in);
508+
if (out) sam_close(out);
485509
}
486510

487511
static void samrecord_layout(void)

0 commit comments

Comments
 (0)