如何修复HP Fortify扫描出的ReadFile缓冲区溢出问题？

Background #

I recently ran HP Fortify on my in-development project, and it flagged a critical potential buffer overflow issue with ReadFile. I'm certain I never pass an empty std::vector<char> buffer, but the scan report says: "The Read() function in serialport.cpp at line 225 may write beyond the bounds of allocated memory, which could lead to data corruption, program crashes, or malicious code execution."

Relevant Code #

DWORD serialport::Read(std::vector<char> & buffer) {
    DWORD read = 0;
    int val = ReadFile(h, &buffer[0], buffer.size(), &read, NULL);
    if (val == 0) {
        LPCSTR ptr = "";
        PrintError(ptr);
    }
    buffer.resize(read);
    return read;
}

Fixes & Analysis #

Hey, let's break down why Fortify is flagging this and how to resolve it. The tool isn't saying you're actively passing empty buffers—it's picking up on potential edge cases that could introduce vulnerabilities down the line, even if your current logic doesn't hit them. Here are actionable fixes:

1. Explicitly Validate Buffer Validity #

Even if you don't pass empty vectors now, there's no guardrail in code to prevent future mistakes. Calling &buffer[0] on an empty vector is undefined behavior, and static analyzers hate that uncertainty. Add a check up front:

DWORD serialport::Read(std::vector<char> & buffer) {
    DWORD read = 0;
    // Guard against empty buffers
    if (buffer.empty()) {
        PrintError("Empty buffer passed to serialport::Read()");
        return 0;
    }
    // Use buffer.data() instead of &buffer[0] for safer C++11+ semantics
    BOOL val = ReadFile(h, buffer.data(), static_cast<DWORD>(buffer.size()), &read, NULL);
    if (!val) {
        PrintError("ReadFile operation failed");
    }
    buffer.resize(read);
    return read;
}

Note I swapped int val for BOOL—ReadFile returns a Windows BOOL type, and using int can confuse static analyzers. buffer.data() is also safer than &buffer[0] for empty vectors (it returns nullptr instead of triggering undefined behavior).

2. Explicitly Cap Read Size (For Analyzer Peace of Mind) #

While ReadFile will never write more bytes than you specify in buffer.size(), Fortify might not recognize this Windows API guarantee. Adding an explicit bounds check can quiet the alarm:

// Right after calling ReadFile
read = std::min(read, static_cast<DWORD>(buffer.size()));
buffer.resize(read);

This is technically redundant for correct ReadFile behavior, but it removes any ambiguity for the static analyzer.

3. Encapsulate Buffer Management (Eliminate External Dependencies) #

A cleaner fix is to take control of the buffer entirely, so callers can't pass invalid buffers in the first place. Rewrite the function to create and return the buffer internally:

std::vector<char> serialport::Read(DWORD maxBytesToRead) {
    std::vector<char> buffer(maxBytesToRead);
    DWORD read = 0;
    BOOL val = ReadFile(h, buffer.data(), maxBytesToRead, &read, NULL);
    if (!val) {
        PrintError("ReadFile operation failed");
    }
    buffer.resize(read);
    return buffer;
}

This way, you're guaranteeing the buffer is always valid when passed to ReadFile, and callers don't have to manage buffer allocation at all.

4. Improve Error Handling #

Your original error handling is a bit redundant (LPCSTR ptr = "";)—make it more useful by capturing the actual Windows error code:

if (!val) {
    DWORD errCode = GetLastError();
    char errMsg[256];
    FormatMessageA(FORMAT_MESSAGE_FROM_SYSTEM | FORMAT_MESSAGE_IGNORE_INSERTS,
                   NULL, errCode, MAKELANGID(LANG_NEUTRAL, SUBLANG_DEFAULT),
                   errMsg, sizeof(errMsg), NULL);
    PrintError(errMsg);
}

This gives you concrete debug info if ReadFile fails, which is way more helpful than an empty string.

内容的提问来源于stack exchange，提问作者agent_bean