Problem in parsing 2 files

[az55555]

Thanks for the great project! I'm trying to use it for reading versions from executables. I've built it with Qt Creator, gcc, qbs (see my .qbs and .cpp files in the archive): pe-parse.zip Most EXE and DLL files I've tried were parsed fine but two of them failed:

• System.Text.Json.dll - getDebugDir fails. Since this file has an Authenticode signature of .NET project, it's unlikely that the file is currupt.
• ConfKarat.exe - incorrect version resource is passed to IterRsrc callback. Windows explorer file properties box shows the version. I can also see the version in a hex editor. But res.buf->buf in my code doesn't point to anything containing VS_VERSION_INFO and 0xfeef04bd signature.

Here is my code:

struct MY_VS_FIXEDFILEINFO
{
    quint32 dwSignature;
    quint32 dwStrucVersion;
    quint32 dwFileVersionMS;
    quint32 dwFileVersionLS;
    quint32 dwProductVersionMS;
    quint32 dwProductVersionLS;
    quint32 dwFileFlagsMask;
    quint32 dwFileFlags;
    quint32 dwFileOS;
    quint32 dwFileType;
    quint32 dwFileSubtype;
    quint32 dwFileDateMS;
    quint32 dwFileDateLS;
};

int extractVersionFromResourceCallback(void* ptr, const peparse::resource& res)
{
    // Skip all resource types execpt RT_VERSION
    if(res.type != 16) return 0;

    // Find VS_FIXEDFILEINFO in the buffer
    auto structSize = sizeof(MY_VS_FIXEDFILEINFO);
    for(size_t offset = 0; offset + structSize <= res.size; offset += 4)
    {
        auto verInfo = (const MY_VS_FIXEDFILEINFO*)(res.buf->buf + offset);
        if (verInfo->dwSignature == 0xfeef04bd)
        {
            QString& result = *(QString*)ptr;
            result = QStringLiteral("%1.%2.%3.%4")
                .arg(( verInfo->dwFileVersionMS >> 16 ) & 0xffff)
                .arg(( verInfo->dwFileVersionMS >>  0 ) & 0xffff)
                .arg(( verInfo->dwFileVersionLS >> 16 ) & 0xffff)
                .arg(( verInfo->dwFileVersionLS >>  0 ) & 0xffff);
            return 1;
        }
    }

    // Maybe there's another version resource we'll understand
    return 0;
}

...

    // Have to use mutex because pe-parse library uses globals
    static QMutex peparseMutex;
    QMutexLocker locker(&peparseMutex);

    // Try to parse PE
    auto peptr = peparse::ParsePEFromPointer((std::uint8_t*)m_data.data(), m_data.size());
    if (peptr == nullptr)
    {
        qWarning() << "Failed to parse PE. Code:" << peparse::GetPEErr() << endl
                   << "Error:" << QString::fromStdString(peparse::GetPEErrString()) << endl
                   << "Location:" << QString::fromStdString(peparse::GetPEErrLoc());
        return QString();
    }

    // Make sure we cleanup properly
    try
    {
        // Iterate resources
        QString result;
        peparse::IterRsrc(peptr, extractVersionFromResourceCallback, &result);
        DestructParsedPE(peptr);
        return result;
    }
    catch (...)
    {
        DestructParsedPE(peptr);
        throw;
    }

[batuzovk]

We had similar issue with getDebugDir on a completely different input file. From the looks of it, IMAGE_DIRECTORY_ENTRY_DEBUG is not something required for execution, and some toolchains put there various non-conforming information they need for their own debugging purposes. In our case it was a string - a path to the project on the build machine. The resulting size of data directory wasn't even divisible by entry size.

Conceptually, it is probably better to ignore errors while parsing non-essential parts like debug data directories. Pe-parse code causing troubles is this:

https://github.com/trailofbits/pe-parse/blob/b43ff3785252c78cdb14f1a841c0fc3a48fdbfd4/pe-parser-library/src/parse.cpp#L1891-L1893

replacing return false; with break; solves the issue. There is another return false; few lines above. It is probably better to replace it with break; as well.

[woodruffw]

@batuzovk I agree that it'd be fine to skip unparseable non-critical sections, as long as we're confident that no other code assumes they've been parsed correctly (since the current error state preserves that invariant). Could you send a PR for this?

[batuzovk]

In this particular case there is code that skips everything starting from unparsable entry already, so we won't change much in this regard.

https://github.com/trailofbits/pe-parse/blob/b43ff3785252c78cdb14f1a841c0fc3a48fdbfd4/pe-parser-library/src/parse.cpp#L1867-L1871

I'll make PR tomorrow.

[batuzovk]

I've opened merge request https://github.com/trailofbits/pe-parse/pull/199 It should address the issue with the first file.