0ca1bd9ec1196e15b2ea7cef508bd7f382bb5add
braney
  Mon Aug 31 13:30:10 2026 -0700
hgc: reject a bigBed type line that declares fewer than three fields, refs #36940

A bigBed always has chrom, chromStart and chromEnd.  bigBedClick trusted the
field count from the type line, so a track hub could declare "type bigBed 1",
"type bigBed 2" or a negative count and reach the item detail page with that
value.  Two things then went wrong.  The variable length array at the top of the
interval loop was sized from it, which is undefined behaviour for a negative
count.  Then restBedFields went negative, so extraFields pointed before the
start of restFields and getExtraFields read off the front of that stack array.
The existing field count check sits twelve lines further down and never got the
chance to stop either one.

Check the count once, right after the zero sentinel has been resolved from the
file, and abort with a message that names the track and the count it declared.
The old path aborted too, just later and with wording that blamed a
disagreement rather than the bad type line.

The check has to come after the zero case is resolved.  Zero means "take the
count from the file", which is why the similar minimum in hgc.c was wrong and
was removed earlier in this ticket.

Tested with hubs declaring -5, 1, 2, 3, 6 and no count.  The first three now
stop with the new message; the last three are unchanged.  Output for the valid
counts is byte identical to a control build from the same tree, and a real
"bigBed 3" track still renders.

diff --git src/hg/hgc/bigBedClick.c src/hg/hgc/bigBedClick.c
index 1641d53466c..fb4e461e271 100644
--- src/hg/hgc/bigBedClick.c
+++ src/hg/hgc/bigBedClick.c
@@ -429,30 +429,37 @@
     ivStart = max(0, start-1);
     ivEnd++;
     }
 char *quickLiftFile = cloneString(trackDbSetting(tdb, "quickLiftUrl"));
 struct hash *chainHash = NULL;
 struct bigBedInterval *bbList = NULL;
 if (quickLiftFile)
     bbList = quickLiftGetIntervals(quickLiftFile, bbi, chrom, ivStart, ivEnd, &chainHash);
 else
     bbList = bigBedIntervalQuery(bbi, chrom, ivStart, ivEnd, 0, lm);
 
 /* Get bedSize if it's not already defined. */
 if (bedSize == 0)
     bedSize = bbi->definedFieldCount;
 
+/* A bigBed always has at least chrom, chromStart and chromEnd.  A smaller count
+ * can only come from a bad type line, and the bedSize - 3 below would then run
+ * off the front of restFields[]. */
+if (bedSize < 3)
+    errAbort("Track %s declares 'type bigBed %d', but a bigBed has at least 3 fields.",
+             tdb->track, bedSize);
+
 char *scoreFilter = cartOrTdbString(cart, tdb, "scoreFilter", NULL);
 int minScore = 0;
 if (scoreFilter)
     minScore = atoi(scoreFilter);
 
 /* Find particular item in list - matching start, and item if possible. */
 boolean found = FALSE;
 boolean firstTime = TRUE;
 struct bigBedInterval *bb;
 for (bb = bbList; bb != NULL; bb = bb->next)
     {
     if (bedSize > 3)
 	{
 	char *name = cloneFirstWordByDelimiterNoSkip(bb->rest, '\t');
 	boolean match = (isEmpty(name) && isEmpty(item)) || sameOk(name, item);