Skip to content

Commit 8b5fc25

Browse files
committed
pff2mt: more robust mv_decode_* checking
1 parent 216871f commit 8b5fc25

File tree

1 file changed

+35
-15
lines changed

1 file changed

+35
-15
lines changed

tools/pff2mt.cpp

Lines changed: 35 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -321,12 +321,12 @@ mv_decode_str(uint32_t proptag, const uint8_t *data, size_t dsize)
321321
return nullptr;
322322
std::unique_ptr<TPROPVAL_ARRAY, gi_delete> tp(static_cast<TPROPVAL_ARRAY *>(malloc(sizeof(TPROPVAL_ARRAY))));
323323
if (tp == nullptr)
324-
return nullptr;
324+
throw std::bad_alloc();
325325
auto pv = static_cast<TAGGED_PROPVAL *>(malloc(sizeof(TAGGED_PROPVAL)));
326326
tp->count = 1;
327327
tp->ppropval = pv;
328328
if (tp->ppropval == nullptr)
329-
return nullptr;
329+
throw std::bad_alloc();
330330
auto ba = static_cast<STRING_ARRAY *>(malloc(sizeof(STRING_ARRAY)));
331331
if (ba == nullptr)
332332
throw std::bad_alloc();
@@ -364,7 +364,7 @@ mv_decode_str(uint32_t proptag, const uint8_t *data, size_t dsize)
364364
}
365365
if (next_ofs < ofs) {
366366
fprintf(stderr, "PF-1069: broken MV data\n");
367-
return nullptr;
367+
break;
368368
}
369369
if (PROP_TYPE(proptag) == PT_MV_STRING8)
370370
ba->ppstr[i] = strndup(reinterpret_cast<const char *>(&data[ofs]), next_ofs - ofs);
@@ -383,27 +383,49 @@ mv_decode_bin(uint32_t proptag, const uint8_t *data, size_t dsize)
383383
return nullptr;
384384
std::unique_ptr<TPROPVAL_ARRAY, gi_delete> tp(static_cast<TPROPVAL_ARRAY *>(malloc(sizeof(TPROPVAL_ARRAY))));
385385
if (tp == nullptr)
386-
return nullptr;
386+
throw std::bad_alloc();
387387
auto pv = static_cast<TAGGED_PROPVAL *>(malloc(sizeof(TAGGED_PROPVAL)));
388388
tp->count = 1;
389389
tp->ppropval = pv;
390390
if (tp->ppropval == nullptr)
391-
return nullptr;
391+
throw std::bad_alloc();
392392
auto ba = static_cast<BINARY_ARRAY *>(malloc(sizeof(BINARY_ARRAY)));
393393
if (ba == nullptr)
394394
throw std::bad_alloc();
395395
pv->proptag = proptag;
396396
pv->pvalue = ba;
397-
ba->count = le32p_to_cpu(data);
398-
ba->pbin = static_cast<BINARY *>(calloc(ba->count, sizeof(BINARY)));
397+
auto nelem = le32p_to_cpu(data);
398+
ba->count = 0;
399+
ba->pbin = static_cast<BINARY *>(calloc(nelem, sizeof(BINARY)));
399400
if (ba->pbin == nullptr)
400401
throw std::bad_alloc();
401-
for (size_t i = 0; i < ba->count; ++i) {
402-
auto ofs = le32p_to_cpu(&data[4*(i+1)]);
403-
auto next_ofs = i == ba->count - 1 ? dsize : le32p_to_cpu(&data[4*(i+2)]);
402+
for (; ba->count < nelem; ++ba->count) {
403+
auto i = ba->count;
404+
uint32_t ofs = 4 * (i + 1), next_ofs = 4 * (i + 2);
405+
if (dsize < next_ofs) {
406+
fprintf(stderr, "PF-1074: broken MV data\n");
407+
break;
408+
}
409+
ofs = le32p_to_cpu(&data[ofs]);
410+
if (dsize < ofs) {
411+
fprintf(stderr, "PF-1075: broken MV data\n");
412+
break;
413+
}
414+
if (i == nelem - 1) {
415+
next_ofs = dsize;
416+
} else if (dsize < next_ofs + 4) {
417+
fprintf(stderr, "PF-1076: broken MV data\n");
418+
break;
419+
} else {
420+
next_ofs = le32p_to_cpu(&data[next_ofs]);
421+
}
422+
if (dsize < next_ofs) {
423+
fprintf(stderr, "PF-1077: broken MV data\n");
424+
break;
425+
}
404426
if (next_ofs < ofs) {
405427
fprintf(stderr, "PF-1068: broken MV data\n");
406-
return nullptr;
428+
break;
407429
}
408430
ba->pbin[i].cb = next_ofs - ofs;
409431
ba->pbin[i].pv = malloc(ba->pbin[i].cb);
@@ -557,13 +579,11 @@ static void recordent_to_tpropval(libpff_record_entry_t *rent, TPROPVAL_ARRAY *a
557579
case PT_MV_STRING8:
558580
case PT_MV_UNICODE:
559581
uextra = mv_decode_str(pv.proptag, buf.get(), dsize);
560-
if (uextra != nullptr)
561-
pv.pvalue = uextra->ppropval[0].pvalue;
582+
pv.pvalue = uextra != nullptr ? uextra->ppropval[0].pvalue : nullptr;
562583
break;
563584
case PT_MV_BINARY:
564585
uextra = mv_decode_bin(pv.proptag, buf.get(), dsize);
565-
if (uextra != nullptr)
566-
pv.pvalue = uextra->ppropval[0].pvalue;
586+
pv.pvalue = uextra != nullptr ? uextra->ppropval[0].pvalue : nullptr;
567587
break;
568588
case PT_MV_CLSID:
569589
u.ga.count = dsize / sizeof(GUID);

0 commit comments

Comments
 (0)