ReadStat icon indicating copy to clipboard operation
ReadStat copied to clipboard

Add a bound check for sas7bcat_parse_value_labels when finding the offset of labels.

Open kiwiwarmnfuzzy opened this issue 1 year ago • 1 comments

lbp1 will try to chase and read as many value labels as label_count_capacity, which may result in accessing memory locations past the allocated region, especially for malformed data. This also results in Address sanitizer error on some inputs. Add a bounds check to avoid it.

Closes #299 Maybe related to #285

kiwiwarmnfuzzy avatar Jul 26 '23 21:07 kiwiwarmnfuzzy

@evanmiller Hi there! Please take a look when you have time - this addresses one of the vulnerabilities identified by our fuzzing tool; realistically, this can happen with incorrect or malformed data.

kiwiwarmnfuzzy avatar Jul 31 '23 21:07 kiwiwarmnfuzzy

Hi, I'm just confused because I thought line 63 was performing this check

        if (&lbp1[3] - value_start > value_labels_len || sas_read2(&lbp1[2], ctx->bswap) < 0) {

But maybe I'm missing something?

evanmiller avatar May 04 '24 12:05 evanmiller

Ah, looks like a regression introduced in #293. I will prepare a fix.

evanmiller avatar May 04 '24 12:05 evanmiller