search

CSB5 / lofreq

LoFreq Star: Sensitive variant calling from sequencing data
http://csb5.github.io/lofreq/
Other
100 stars 30 forks source link

Fix segfault due to ";HRUN=3" INFO field #95

Closed jmarshall closed 4 years ago

jmarshall commented 4 years ago

Fix vcf_var_has_info_key()'s key comparison, which causes the crash in #89 due to ;HRUN=3:

NC_045512   1044    .   CA  C   94  PASS    ;HRUN=3

Previously any key would be returned as being matched by the entirely empty "key=value" at the start of ";HRUN=3". Note that INFO is semicolon-separated, so this is in fact invalid VCF.

This resulted in a crash when e.g. lofreq_filter.c looking for "SB" expected a value but in this case the returned sb_char was NULL. Crash fixed by no longer returning a hit for "SB" in this case.

(Also convert a couple of previously missed legacy samtools API usages. Oops.)

andreas-wilm commented 4 years ago

Thanks for reporting John! I think I found the culprit for the truncated info field (I used the same char* twice in sprintf, which leads to undefined results according to the POSIX standard; sorry this was shoddily written). Just committed a fix with 8206db2 (more exactly 1e44677). Your changes make certainly sense in addition. Will merge as soon as it's confirmed that this was indeed the culprit. Many thanks again

jmarshall commented 4 years ago

Yep, your fix looks okay and stops the malformed ;HRUN=3 from occurring. Applying both fixes hits the problem from both sides…

I don't think there was anything undefined about the sprintfs; it was just a bug in that it overwrote the text that you had already put in buf.

jmarshall commented 4 years ago

…pushed a more complete fix for the don't recognise AD=1;SBNOT=2;HRUN=3 as SB=2 part.

andreas-wilm commented 4 years ago

Thanks again @jmarshall and @bgruening !