6c676a0e9ed642676008731762141e0c16822787 braney Sat Aug 22 09:31:14 2026 -0700 vcf: tighten validation of the genotype allele index, refs #38155 New parseAlleleIx returns missing data for a GT allele index that the record has no allele for, and for a value that will not fit the field. It checks the value as an int, before it reaches the field. vcfParseGenotypes and vcfParseGenotypesGtOnly both go through it, and a check after the per-genotype loop covers the PL and SGT fallbacks, which choose an index without consulting the allele count. hapIxA and hapIxB become signed char. Plain char is unsigned on some architectures, and the negative missing-data value did not stay negative there. Size and field offsets are unchanged, and x86 code generation is identical. Adds a lib/tests case for malformed and boundary genotype indexes. diff --git src/lib/vcf.c src/lib/vcf.c index c2800efef96..723ce9d72f5 100644 --- src/lib/vcf.c +++ src/lib/vcf.c @@ -1274,48 +1274,56 @@ { if (sameString(key, def->key)) return def; } return NULL; } static enum vcfInfoType typeForGtFormat(struct vcfFile *vcff, const char *key) /* Look up the type of FORMAT component key, in the definitions from the header, * and failing that, from the keys reserved in the spec. */ { struct vcfInfoDef *def = vcfInfoDefForGtKey(vcff, key); return def ? def->type : vcfInfoString; } -static void parseGt(char *genotype, struct vcfGenotype *gt) +static signed char parseAlleleIx(char *string, int alleleCount) +/* Parse one allele index out of a GT field. Return -1 for missing data, and also for an + * index that this record has no allele for, so that every caller sees either a real allele + * or missing data. */ +{ +if (string[0] == '.') + return -1; +int alleleIx = atoi(string); +if (alleleIx < 0 || alleleIx >= alleleCount) + return -1; +return alleleIx; +} + +static void parseGt(char *genotype, struct vcfGenotype *gt, int alleleCount) /* Parse genotype, which should be something like "0/0", "0/1", "1|0" or "0/." into gt fields. */ { char *sep = strchr(genotype, '|'); if (sep != NULL) gt->isPhased = TRUE; else sep = strchr(genotype, '/'); -if (genotype[0] == '.') - gt->hapIxA = -1; -else - gt->hapIxA = atoi(genotype); +gt->hapIxA = parseAlleleIx(genotype, alleleCount); if (sep == NULL) gt->isHaploid = TRUE; -else if (sep[1] == '.') - gt->hapIxB = -1; else - gt->hapIxB = atoi(sep+1); + gt->hapIxB = parseAlleleIx(sep+1, alleleCount); } static void parseSgtAsGt(struct vcfRecord *rec, struct vcfGenotype *gt) /* Parse SGT to normal and tumor genotypes */ { const struct vcfInfoElement *sgtEl = vcfRecordFindInfo(rec, "SGT"); if (sgtEl) { char *val = sgtEl->values[0].datString; // set hapIxA and hapIxB where 0 = ref, 1 = alt if (startsWith("ref->", val)) { //indel, use 0/0 for normal and 0/1 for tumor if (sameString(gt->id, "NORMAL")) gt->hapIxA = gt->hapIxB = 0; @@ -1459,83 +1467,89 @@ vcfFileErr(vcff, "The FORMAT column has %d words but the genotype column for %s " "has %d words", formatWordCount, vcff->genotypeIds[i], gtWordCount); if (gtWordCount > formatWordCount) gtWordCount = formatWordCount; gt->id = vcff->genotypeIds[i]; gt->infoCount = gtWordCount; gt->infoElements = vcfFileAlloc(vcff, gtWordCount * sizeof(struct vcfInfoElement)); gt->hapIxA = gt->hapIxB = -1; boolean foundGT = FALSE; int j; for (j = 0; j < gtWordCount; j++) { // Special parsing of genotype: if (sameString(formatWords[j], vcfGtGenotype)) { - parseGt(gtWords[j], gt); + parseGt(gtWords[j], gt, record->alleleCount); foundGT = TRUE; } else if (!foundGT && sameString(formatWords[j], vcfGtPhred)) { parsePlAsGt(gtWords[j], gt); foundGT = TRUE; } struct vcfInfoElement *el = &(gt->infoElements[j]); el->key = formatWords[j]; el->count = parseInfoValue(record, formatWords[j], formatTypes[j], gtWords[j], &(el->values), &(el->missingData)); if (el->count >= VCF_MAX_INFO) vcfFileErr(vcff, "A single element of the genotype column for \"%s\" " "has at least %d values; " "VCF_MAX_INFO may need to be increased in vcf.c!", gt->id, VCF_MAX_INFO); } if (!foundGT) //try SGT { parseSgtAsGt(record, gt); if (gt->hapIxA != -1) foundGT = TRUE; } if (i == 0 && !foundGT) vcfFileErr(vcff, "Genotype FORMAT column includes neither GT nor PL; unable to parse genotypes."); + // The PL and SGT fallbacks above pick allele indexes without consulting alleleCount, + // so they too can name an allele that this record does not have. + if (gt->hapIxA >= record->alleleCount) + gt->hapIxA = -1; + if (gt->hapIxB >= record->alleleCount) + gt->hapIxB = -1; } record->genotypeUnparsedStrings = NULL; } void vcfParseGenotypesGtOnly(struct vcfRecord *record) /* Translate record->genotypesUnparsedStrings[] into proper struct vcfGenotype[], but ignore * genotype info elements, IDs, etc; parse only the genotypes (e.g. for quick display in hgTracks). * This destroys genotypesUnparsedStrings. */ { if (record->genotypeUnparsedStrings == NULL) return; struct vcfFile *vcff = record->file; record->genotypes = vcfFileAlloc(vcff, vcff->genotypeCount * sizeof(struct vcfGenotype)); int i; for (i = 0; i < vcff->genotypeCount; i++) { // Parse (.|[0-9])([/|](.|[0-9]))? in first one to four characters char *string = record->genotypeUnparsedStrings[i]; struct vcfGenotype *gt = &(record->genotypes[i]); gt->hapIxA = gt->hapIxB = -1; if (string[0] != '.' && isdigit(string[0])) - gt->hapIxA = atoi(string); + gt->hapIxA = parseAlleleIx(string, record->alleleCount); if (string[1] == '/' || string[1] == '|') { if (isdigit(string[2])) - gt->hapIxB = atoi(string+2); + gt->hapIxB = parseAlleleIx(string+2, record->alleleCount); if (string[1] == '|') gt->isPhased = TRUE; } else gt->isHaploid = TRUE; } record->genotypeUnparsedStrings = NULL; } const struct vcfGenotype *vcfRecordFindGenotype(struct vcfRecord *record, char *sampleId) /* Find the genotype and associated info for the individual, or return NULL. * This calls vcfParseGenotypes if it has not already been called. */ { struct vcfFile *vcff = record->file; if (sampleId == NULL || vcff->genotypeCount == 0)