JusticeRage / Manalyze

A static analyzer for PE executables.
GNU General Public License v3.0
1.01k stars 160 forks source link

crash in PE::_parse_exports(FILE* f) #1

Closed rwfpl closed 8 years ago

rwfpl commented 8 years ago
boost::scoped_array<boost::uint32_t> names(new boost::uint32_t[ied.NumberOfNames]);
boost::scoped_array<boost::uint16_t> ords(new boost::uint16_t[ied.NumberOfNames]);

if ied.NumberOfNames is sufficently big, there will be unhandled bad alloc exception. Sample (pass: infected): https://mega.nz/#!V5pEgARQ!qwQsCH3enmjnv9--x_d4WDYyZpja1au2NYokzKKT7sQ

rwfpl commented 8 years ago
    for (unsigned int i = 0 ; i < ied.NumberOfNames ; ++i)
    {
        offset = _rva_to_offset(names[i]);
        if (!offset || ords[i] > _exports.size() || !utils::read_string_at_offset(f, offset, _exports.at(ords[i])->Name))
        {
            PRINT_ERROR << "Could not match an export name with its address!" << std::endl;
            return false;
        }
    }

content of ords[] array is read from file and not verified, thus it is possible to access _exports vector out of its range. Sample (pass: infected): https://mega.nz/#!c8BEXLQL!ryY2CTd_koj6y-ol35OOWO4g1cDyDBxuLhrUimhQx3c

JusticeRage commented 8 years ago

Issue 1 has been fixed by catching the exception. You were right about issue 2 too: the test should have been: if (!offset || ords[i] >= _exports.size() || ... instead of if (!offset || ords[i] > _exports.size() || .... This has also been fixed in the latest commit.