HyperDbg/hyperdbg/libhyperdbg/code/debugger/misc/pci-id.cpp

478 lines
14 KiB
C++
Raw Permalink Normal View History

/**
* @file pci-id.cpp
* @author Bj<EFBFBD>rn Ruytenberg (bjorn@bjornweb.nl)
* @brief Provides runtime access to PCI ID database
* @details
* @version 0.12
* @date 2024-12-04
*
* @copyright This project is released under the GNU Public License v3.
*
*/
#include "pch.h"
static CHAR * PciIdDatabaseBuffer = NULL;
/**
* @brief Trims whitespaces in passed string
*
* @param Str
* @param MaxLen
* @return CHAR*
*/
CHAR *
TrimWhitespace(CHAR * Str, UINT8 MaxLen)
{
CHAR * End;
while (*Str == ' ')
Str++; // Trim leading space
if (*Str == '\0')
return Str;
End = Str + PlatformStrnlen(Str, MaxLen) - 1;
while (End > Str && (*End == ' ' || *End == '\n' || *End == '\r'))
End--;
*(End + 1) = '\0';
return Str;
}
/**
* @brief Converts passed string to lowercase
*
* @param Str
* @return CHAR*
*/
CHAR *
ToLower(CHAR * Str)
{
UINT8 StrLength = (UINT8)PlatformStrnlen(Str, PCI_ID_AS_STR_LENGTH);
CHAR * CurrentChar = Str;
while (CurrentChar < Str + StrLength)
{
*CurrentChar = tolower(*CurrentChar);
CurrentChar++;
}
return Str;
}
/**
* @brief Read line from string. Treats SrcBuffer as a stream (similar to fgets and friends), i.e. updates SrcBuffer by number of characters read.
*
* @param DestBuffer
* @param CharLimit
* @param SrcBuffer
* @return CHAR*
*/
CHAR *
ReadLine(CHAR * DestBuffer, UINT64 CharLimit, CHAR ** SrcBuffer)
{
CHAR * Line = strchr(*SrcBuffer, '\n');
if (!Line)
{
return NULL;
}
else
{
Fix memory-safety and robustness issues in script engine and PCI ID parser Code audit of the script engine's scanner/token handling and of the PCI ID database parser. Each of the issues below was reproduced against the current code before the fix and re-checked afterwards. script-engine/scanner.c * An unterminated string literal ("abc or L"abc) hung the scanner in an endless loop: sgetc() returns EOF without consuming input, and neither string loop tested for it, so the token grew until allocation failed. Both loops now stop at EOF and report the token as UNKNOWN. script-engine/common.c * AppendByte()/AppendWchar() doubled Token->MaxLen before checking whether the larger buffer was actually allocated. After a failed allocation MaxLen described memory that did not exist and the next append wrote past the end of the old buffer. MaxLen is now committed only on success. * CopyToken() allocated strlen(Value) + 1 bytes but carried over the source token's Len and MaxLen, so the copy's advertised capacity did not match its allocation, and WSTRING payloads were truncated at their first embedded null byte. The copy is now sized from Len/MaxLen and copied by length, with a fallback to the string length for the grammar tokens in parse-table.c, which only initialize Type and Value. * NewToken() set MaxLen to the value length, which is zero for an empty value. The 'Len >= MaxLen - 1' test in the append routines is unsigned, so a zero MaxLen wrapped and disabled buffer growth entirely. * IsUnderscore() tested 'c >= '_'', which also accepted the backtick, the lowercase letters, '{', '|', '}', '~' and DEL. Register scanning uses it, so '@rax|1' was lexed as one malformed register name instead of a register, an operator and a number. The pseudo-register path already compared against '_' directly. * NewTokenList() did not check the allocation of its Head buffer. * NewTemp() kept the last handed-out id in a static, so an exhausted temp list produced a token aliasing a temporary still in use, and it derived MaxTempNumber from an out-of-range index. It also dereferenced the new token without a null check. * FreeTemp() indexed the MAX_TEMP_COUNT-entry map with an unchecked value parsed out of the token text. * RotateLeftStringOnce() wrote to str[-1] when handed an empty string. libhyperdbg/debugger/misc/pci-id.cpp * The database file was read into a malloc(Length) buffer that was never null-terminated, while ReadLine() walks it with strchr(). Looking up an absent vendor scans to the end and reads past the allocation. * The matched Vendor was allocated with malloc() and its Devices list head was only assigned once a device line was parsed, so a vendor with no device entries left it uninitialized and FreeVendor() walked a garbage pointer. * FreeVendor() released the device and subdevice lists but never the Vendor itself, leaking one per lookup for every enumerated PCI device. * The file handle leaked when the buffer allocation failed, ftell() and fread() results were unused, and several error paths leaked the Vendor or the not-yet-linked Device/SubDevice. * strncmp() compared sizeof(VendorId) bytes, which is the size of the pointer rather than the length of a vendor id. * ReadLine() passed an unclamped count to strncpy_s(), which triggers the invalid parameter handler for a line longer than the destination. * GetVendorById() ignored the GetModuleFileName() result and overwrote the tail of the path buffer without checking the room left in it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01W3C1DuhHtqK64eEkHjKCHM
2026-08-01 14:22:52 +00:00
//
// The copy length is clamped to the destination, otherwise a line longer than
// CharLimit makes strncpy_s() invoke the invalid parameter handler
//
SIZE_T LineLength = (SIZE_T)(Line - *SrcBuffer);
if (LineLength > CharLimit - 1)
{
LineLength = (SIZE_T)(CharLimit - 1);
}
PlatformStrNCpy(DestBuffer, (SIZE_T)CharLimit, *SrcBuffer, LineLength);
*SrcBuffer += (Line - *SrcBuffer + 1);
return *SrcBuffer;
}
}
/**
* @brief Get Vendor by PCI ID, encoded in ASCII. Do not call directly - use GetVendorById() instead.
*
* @param Filename
* @param VendorId
* @return Vendor *
*/
Vendor *
GetVendorByIdStr(const CHAR * Filename, const CHAR * VendorId)
{
Vendor * MatchedVendor = NULL;
BOOLEAN FoundVendorId = FALSE;
Device * LastDevice = NULL;
SubDevice * LastSubDevice = NULL;
CHAR * PciIdDbBufPtr = NULL;
CHAR Line[1024] = {'\0'};
if (!PciIdDatabaseBuffer)
{
FILE * f = fopen(Filename, "rb");
SIZE_T Length = 0;
if (f == NULL)
{
ShowMessages("err, cannot open file '%s' (error 0x%x)\n", Filename, errno);
return NULL;
}
fseek(f, 0, SEEK_END);
Fix memory-safety and robustness issues in script engine and PCI ID parser Code audit of the script engine's scanner/token handling and of the PCI ID database parser. Each of the issues below was reproduced against the current code before the fix and re-checked afterwards. script-engine/scanner.c * An unterminated string literal ("abc or L"abc) hung the scanner in an endless loop: sgetc() returns EOF without consuming input, and neither string loop tested for it, so the token grew until allocation failed. Both loops now stop at EOF and report the token as UNKNOWN. script-engine/common.c * AppendByte()/AppendWchar() doubled Token->MaxLen before checking whether the larger buffer was actually allocated. After a failed allocation MaxLen described memory that did not exist and the next append wrote past the end of the old buffer. MaxLen is now committed only on success. * CopyToken() allocated strlen(Value) + 1 bytes but carried over the source token's Len and MaxLen, so the copy's advertised capacity did not match its allocation, and WSTRING payloads were truncated at their first embedded null byte. The copy is now sized from Len/MaxLen and copied by length, with a fallback to the string length for the grammar tokens in parse-table.c, which only initialize Type and Value. * NewToken() set MaxLen to the value length, which is zero for an empty value. The 'Len >= MaxLen - 1' test in the append routines is unsigned, so a zero MaxLen wrapped and disabled buffer growth entirely. * IsUnderscore() tested 'c >= '_'', which also accepted the backtick, the lowercase letters, '{', '|', '}', '~' and DEL. Register scanning uses it, so '@rax|1' was lexed as one malformed register name instead of a register, an operator and a number. The pseudo-register path already compared against '_' directly. * NewTokenList() did not check the allocation of its Head buffer. * NewTemp() kept the last handed-out id in a static, so an exhausted temp list produced a token aliasing a temporary still in use, and it derived MaxTempNumber from an out-of-range index. It also dereferenced the new token without a null check. * FreeTemp() indexed the MAX_TEMP_COUNT-entry map with an unchecked value parsed out of the token text. * RotateLeftStringOnce() wrote to str[-1] when handed an empty string. libhyperdbg/debugger/misc/pci-id.cpp * The database file was read into a malloc(Length) buffer that was never null-terminated, while ReadLine() walks it with strchr(). Looking up an absent vendor scans to the end and reads past the allocation. * The matched Vendor was allocated with malloc() and its Devices list head was only assigned once a device line was parsed, so a vendor with no device entries left it uninitialized and FreeVendor() walked a garbage pointer. * FreeVendor() released the device and subdevice lists but never the Vendor itself, leaking one per lookup for every enumerated PCI device. * The file handle leaked when the buffer allocation failed, ftell() and fread() results were unused, and several error paths leaked the Vendor or the not-yet-linked Device/SubDevice. * strncmp() compared sizeof(VendorId) bytes, which is the size of the pointer rather than the length of a vendor id. * ReadLine() passed an unclamped count to strncpy_s(), which triggers the invalid parameter handler for a line longer than the destination. * GetVendorById() ignored the GetModuleFileName() result and overwrote the tail of the path buffer without checking the room left in it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01W3C1DuhHtqK64eEkHjKCHM
2026-08-01 14:22:52 +00:00
LONG FileSize = ftell(f);
if (FileSize < 0)
{
ShowMessages("err, cannot determine the size of file '%s' (error: 0x%x)\n", Filename, errno);
Fix memory-safety and robustness issues in script engine and PCI ID parser Code audit of the script engine's scanner/token handling and of the PCI ID database parser. Each of the issues below was reproduced against the current code before the fix and re-checked afterwards. script-engine/scanner.c * An unterminated string literal ("abc or L"abc) hung the scanner in an endless loop: sgetc() returns EOF without consuming input, and neither string loop tested for it, so the token grew until allocation failed. Both loops now stop at EOF and report the token as UNKNOWN. script-engine/common.c * AppendByte()/AppendWchar() doubled Token->MaxLen before checking whether the larger buffer was actually allocated. After a failed allocation MaxLen described memory that did not exist and the next append wrote past the end of the old buffer. MaxLen is now committed only on success. * CopyToken() allocated strlen(Value) + 1 bytes but carried over the source token's Len and MaxLen, so the copy's advertised capacity did not match its allocation, and WSTRING payloads were truncated at their first embedded null byte. The copy is now sized from Len/MaxLen and copied by length, with a fallback to the string length for the grammar tokens in parse-table.c, which only initialize Type and Value. * NewToken() set MaxLen to the value length, which is zero for an empty value. The 'Len >= MaxLen - 1' test in the append routines is unsigned, so a zero MaxLen wrapped and disabled buffer growth entirely. * IsUnderscore() tested 'c >= '_'', which also accepted the backtick, the lowercase letters, '{', '|', '}', '~' and DEL. Register scanning uses it, so '@rax|1' was lexed as one malformed register name instead of a register, an operator and a number. The pseudo-register path already compared against '_' directly. * NewTokenList() did not check the allocation of its Head buffer. * NewTemp() kept the last handed-out id in a static, so an exhausted temp list produced a token aliasing a temporary still in use, and it derived MaxTempNumber from an out-of-range index. It also dereferenced the new token without a null check. * FreeTemp() indexed the MAX_TEMP_COUNT-entry map with an unchecked value parsed out of the token text. * RotateLeftStringOnce() wrote to str[-1] when handed an empty string. libhyperdbg/debugger/misc/pci-id.cpp * The database file was read into a malloc(Length) buffer that was never null-terminated, while ReadLine() walks it with strchr(). Looking up an absent vendor scans to the end and reads past the allocation. * The matched Vendor was allocated with malloc() and its Devices list head was only assigned once a device line was parsed, so a vendor with no device entries left it uninitialized and FreeVendor() walked a garbage pointer. * FreeVendor() released the device and subdevice lists but never the Vendor itself, leaking one per lookup for every enumerated PCI device. * The file handle leaked when the buffer allocation failed, ftell() and fread() results were unused, and several error paths leaked the Vendor or the not-yet-linked Device/SubDevice. * strncmp() compared sizeof(VendorId) bytes, which is the size of the pointer rather than the length of a vendor id. * ReadLine() passed an unclamped count to strncpy_s(), which triggers the invalid parameter handler for a line longer than the destination. * GetVendorById() ignored the GetModuleFileName() result and overwrote the tail of the path buffer without checking the room left in it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01W3C1DuhHtqK64eEkHjKCHM
2026-08-01 14:22:52 +00:00
fclose(f);
return NULL;
}
Length = (SIZE_T)FileSize;
//
// One extra byte is allocated for the null terminator, as the buffer is later
// walked with strchr() by ReadLine() and would otherwise be read past its end
//
PciIdDatabaseBuffer = (CHAR *)malloc(Length + 1);
if (!PciIdDatabaseBuffer)
{
Fix memory-safety and robustness issues in script engine and PCI ID parser Code audit of the script engine's scanner/token handling and of the PCI ID database parser. Each of the issues below was reproduced against the current code before the fix and re-checked afterwards. script-engine/scanner.c * An unterminated string literal ("abc or L"abc) hung the scanner in an endless loop: sgetc() returns EOF without consuming input, and neither string loop tested for it, so the token grew until allocation failed. Both loops now stop at EOF and report the token as UNKNOWN. script-engine/common.c * AppendByte()/AppendWchar() doubled Token->MaxLen before checking whether the larger buffer was actually allocated. After a failed allocation MaxLen described memory that did not exist and the next append wrote past the end of the old buffer. MaxLen is now committed only on success. * CopyToken() allocated strlen(Value) + 1 bytes but carried over the source token's Len and MaxLen, so the copy's advertised capacity did not match its allocation, and WSTRING payloads were truncated at their first embedded null byte. The copy is now sized from Len/MaxLen and copied by length, with a fallback to the string length for the grammar tokens in parse-table.c, which only initialize Type and Value. * NewToken() set MaxLen to the value length, which is zero for an empty value. The 'Len >= MaxLen - 1' test in the append routines is unsigned, so a zero MaxLen wrapped and disabled buffer growth entirely. * IsUnderscore() tested 'c >= '_'', which also accepted the backtick, the lowercase letters, '{', '|', '}', '~' and DEL. Register scanning uses it, so '@rax|1' was lexed as one malformed register name instead of a register, an operator and a number. The pseudo-register path already compared against '_' directly. * NewTokenList() did not check the allocation of its Head buffer. * NewTemp() kept the last handed-out id in a static, so an exhausted temp list produced a token aliasing a temporary still in use, and it derived MaxTempNumber from an out-of-range index. It also dereferenced the new token without a null check. * FreeTemp() indexed the MAX_TEMP_COUNT-entry map with an unchecked value parsed out of the token text. * RotateLeftStringOnce() wrote to str[-1] when handed an empty string. libhyperdbg/debugger/misc/pci-id.cpp * The database file was read into a malloc(Length) buffer that was never null-terminated, while ReadLine() walks it with strchr(). Looking up an absent vendor scans to the end and reads past the allocation. * The matched Vendor was allocated with malloc() and its Devices list head was only assigned once a device line was parsed, so a vendor with no device entries left it uninitialized and FreeVendor() walked a garbage pointer. * FreeVendor() released the device and subdevice lists but never the Vendor itself, leaking one per lookup for every enumerated PCI device. * The file handle leaked when the buffer allocation failed, ftell() and fread() results were unused, and several error paths leaked the Vendor or the not-yet-linked Device/SubDevice. * strncmp() compared sizeof(VendorId) bytes, which is the size of the pointer rather than the length of a vendor id. * ReadLine() passed an unclamped count to strncpy_s(), which triggers the invalid parameter handler for a line longer than the destination. * GetVendorById() ignored the GetModuleFileName() result and overwrote the tail of the path buffer without checking the room left in it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01W3C1DuhHtqK64eEkHjKCHM
2026-08-01 14:22:52 +00:00
fclose(f);
return NULL;
}
fseek(f, 0, SEEK_SET);
Fix memory-safety and robustness issues in script engine and PCI ID parser Code audit of the script engine's scanner/token handling and of the PCI ID database parser. Each of the issues below was reproduced against the current code before the fix and re-checked afterwards. script-engine/scanner.c * An unterminated string literal ("abc or L"abc) hung the scanner in an endless loop: sgetc() returns EOF without consuming input, and neither string loop tested for it, so the token grew until allocation failed. Both loops now stop at EOF and report the token as UNKNOWN. script-engine/common.c * AppendByte()/AppendWchar() doubled Token->MaxLen before checking whether the larger buffer was actually allocated. After a failed allocation MaxLen described memory that did not exist and the next append wrote past the end of the old buffer. MaxLen is now committed only on success. * CopyToken() allocated strlen(Value) + 1 bytes but carried over the source token's Len and MaxLen, so the copy's advertised capacity did not match its allocation, and WSTRING payloads were truncated at their first embedded null byte. The copy is now sized from Len/MaxLen and copied by length, with a fallback to the string length for the grammar tokens in parse-table.c, which only initialize Type and Value. * NewToken() set MaxLen to the value length, which is zero for an empty value. The 'Len >= MaxLen - 1' test in the append routines is unsigned, so a zero MaxLen wrapped and disabled buffer growth entirely. * IsUnderscore() tested 'c >= '_'', which also accepted the backtick, the lowercase letters, '{', '|', '}', '~' and DEL. Register scanning uses it, so '@rax|1' was lexed as one malformed register name instead of a register, an operator and a number. The pseudo-register path already compared against '_' directly. * NewTokenList() did not check the allocation of its Head buffer. * NewTemp() kept the last handed-out id in a static, so an exhausted temp list produced a token aliasing a temporary still in use, and it derived MaxTempNumber from an out-of-range index. It also dereferenced the new token without a null check. * FreeTemp() indexed the MAX_TEMP_COUNT-entry map with an unchecked value parsed out of the token text. * RotateLeftStringOnce() wrote to str[-1] when handed an empty string. libhyperdbg/debugger/misc/pci-id.cpp * The database file was read into a malloc(Length) buffer that was never null-terminated, while ReadLine() walks it with strchr(). Looking up an absent vendor scans to the end and reads past the allocation. * The matched Vendor was allocated with malloc() and its Devices list head was only assigned once a device line was parsed, so a vendor with no device entries left it uninitialized and FreeVendor() walked a garbage pointer. * FreeVendor() released the device and subdevice lists but never the Vendor itself, leaking one per lookup for every enumerated PCI device. * The file handle leaked when the buffer allocation failed, ftell() and fread() results were unused, and several error paths leaked the Vendor or the not-yet-linked Device/SubDevice. * strncmp() compared sizeof(VendorId) bytes, which is the size of the pointer rather than the length of a vendor id. * ReadLine() passed an unclamped count to strncpy_s(), which triggers the invalid parameter handler for a line longer than the destination. * GetVendorById() ignored the GetModuleFileName() result and overwrote the tail of the path buffer without checking the room left in it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01W3C1DuhHtqK64eEkHjKCHM
2026-08-01 14:22:52 +00:00
SIZE_T BytesRead = fread(PciIdDatabaseBuffer, 1, Length, f);
fclose(f);
Fix memory-safety and robustness issues in script engine and PCI ID parser Code audit of the script engine's scanner/token handling and of the PCI ID database parser. Each of the issues below was reproduced against the current code before the fix and re-checked afterwards. script-engine/scanner.c * An unterminated string literal ("abc or L"abc) hung the scanner in an endless loop: sgetc() returns EOF without consuming input, and neither string loop tested for it, so the token grew until allocation failed. Both loops now stop at EOF and report the token as UNKNOWN. script-engine/common.c * AppendByte()/AppendWchar() doubled Token->MaxLen before checking whether the larger buffer was actually allocated. After a failed allocation MaxLen described memory that did not exist and the next append wrote past the end of the old buffer. MaxLen is now committed only on success. * CopyToken() allocated strlen(Value) + 1 bytes but carried over the source token's Len and MaxLen, so the copy's advertised capacity did not match its allocation, and WSTRING payloads were truncated at their first embedded null byte. The copy is now sized from Len/MaxLen and copied by length, with a fallback to the string length for the grammar tokens in parse-table.c, which only initialize Type and Value. * NewToken() set MaxLen to the value length, which is zero for an empty value. The 'Len >= MaxLen - 1' test in the append routines is unsigned, so a zero MaxLen wrapped and disabled buffer growth entirely. * IsUnderscore() tested 'c >= '_'', which also accepted the backtick, the lowercase letters, '{', '|', '}', '~' and DEL. Register scanning uses it, so '@rax|1' was lexed as one malformed register name instead of a register, an operator and a number. The pseudo-register path already compared against '_' directly. * NewTokenList() did not check the allocation of its Head buffer. * NewTemp() kept the last handed-out id in a static, so an exhausted temp list produced a token aliasing a temporary still in use, and it derived MaxTempNumber from an out-of-range index. It also dereferenced the new token without a null check. * FreeTemp() indexed the MAX_TEMP_COUNT-entry map with an unchecked value parsed out of the token text. * RotateLeftStringOnce() wrote to str[-1] when handed an empty string. libhyperdbg/debugger/misc/pci-id.cpp * The database file was read into a malloc(Length) buffer that was never null-terminated, while ReadLine() walks it with strchr(). Looking up an absent vendor scans to the end and reads past the allocation. * The matched Vendor was allocated with malloc() and its Devices list head was only assigned once a device line was parsed, so a vendor with no device entries left it uninitialized and FreeVendor() walked a garbage pointer. * FreeVendor() released the device and subdevice lists but never the Vendor itself, leaking one per lookup for every enumerated PCI device. * The file handle leaked when the buffer allocation failed, ftell() and fread() results were unused, and several error paths leaked the Vendor or the not-yet-linked Device/SubDevice. * strncmp() compared sizeof(VendorId) bytes, which is the size of the pointer rather than the length of a vendor id. * ReadLine() passed an unclamped count to strncpy_s(), which triggers the invalid parameter handler for a line longer than the destination. * GetVendorById() ignored the GetModuleFileName() result and overwrote the tail of the path buffer without checking the room left in it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01W3C1DuhHtqK64eEkHjKCHM
2026-08-01 14:22:52 +00:00
PciIdDatabaseBuffer[BytesRead] = '\0';
}
PciIdDbBufPtr = PciIdDatabaseBuffer;
while (ReadLine(Line, sizeof(Line), &PciIdDbBufPtr) != NULL)
{
CHAR FormatStr[24];
// Skip comments and empty lines
if (Line[0] == '#' || Line[0] == '\0')
{
continue;
}
// Find vendor
// We assume PCI ID database comprises unique entries only, i.e. we return the first matching entry
if (Line[0] != '\t' && FoundVendorId == FALSE)
{
CHAR VendorBuf[PCI_ID_AS_STR_LENGTH + 1], VendorNameBuf[PCI_NAME_STR_LENGTH + 1];
snprintf(FormatStr, sizeof(FormatStr), "%%4s %%%d[^\n]", PCI_NAME_STR_LENGTH); // FormatStr = "%4s %PCI_NAME_STR_LENGTH[^\n]"
if (sscanf(Line, FormatStr, VendorBuf, VendorNameBuf) == 2)
{
Fix memory-safety and robustness issues in script engine and PCI ID parser Code audit of the script engine's scanner/token handling and of the PCI ID database parser. Each of the issues below was reproduced against the current code before the fix and re-checked afterwards. script-engine/scanner.c * An unterminated string literal ("abc or L"abc) hung the scanner in an endless loop: sgetc() returns EOF without consuming input, and neither string loop tested for it, so the token grew until allocation failed. Both loops now stop at EOF and report the token as UNKNOWN. script-engine/common.c * AppendByte()/AppendWchar() doubled Token->MaxLen before checking whether the larger buffer was actually allocated. After a failed allocation MaxLen described memory that did not exist and the next append wrote past the end of the old buffer. MaxLen is now committed only on success. * CopyToken() allocated strlen(Value) + 1 bytes but carried over the source token's Len and MaxLen, so the copy's advertised capacity did not match its allocation, and WSTRING payloads were truncated at their first embedded null byte. The copy is now sized from Len/MaxLen and copied by length, with a fallback to the string length for the grammar tokens in parse-table.c, which only initialize Type and Value. * NewToken() set MaxLen to the value length, which is zero for an empty value. The 'Len >= MaxLen - 1' test in the append routines is unsigned, so a zero MaxLen wrapped and disabled buffer growth entirely. * IsUnderscore() tested 'c >= '_'', which also accepted the backtick, the lowercase letters, '{', '|', '}', '~' and DEL. Register scanning uses it, so '@rax|1' was lexed as one malformed register name instead of a register, an operator and a number. The pseudo-register path already compared against '_' directly. * NewTokenList() did not check the allocation of its Head buffer. * NewTemp() kept the last handed-out id in a static, so an exhausted temp list produced a token aliasing a temporary still in use, and it derived MaxTempNumber from an out-of-range index. It also dereferenced the new token without a null check. * FreeTemp() indexed the MAX_TEMP_COUNT-entry map with an unchecked value parsed out of the token text. * RotateLeftStringOnce() wrote to str[-1] when handed an empty string. libhyperdbg/debugger/misc/pci-id.cpp * The database file was read into a malloc(Length) buffer that was never null-terminated, while ReadLine() walks it with strchr(). Looking up an absent vendor scans to the end and reads past the allocation. * The matched Vendor was allocated with malloc() and its Devices list head was only assigned once a device line was parsed, so a vendor with no device entries left it uninitialized and FreeVendor() walked a garbage pointer. * FreeVendor() released the device and subdevice lists but never the Vendor itself, leaking one per lookup for every enumerated PCI device. * The file handle leaked when the buffer allocation failed, ftell() and fread() results were unused, and several error paths leaked the Vendor or the not-yet-linked Device/SubDevice. * strncmp() compared sizeof(VendorId) bytes, which is the size of the pointer rather than the length of a vendor id. * ReadLine() passed an unclamped count to strncpy_s(), which triggers the invalid parameter handler for a line longer than the destination. * GetVendorById() ignored the GetModuleFileName() result and overwrote the tail of the path buffer without checking the room left in it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01W3C1DuhHtqK64eEkHjKCHM
2026-08-01 14:22:52 +00:00
//
// VendorId is a pointer, so sizeof() on it yielded the pointer size
// rather than the length of a PCI vendor id
//
if (strncmp(VendorBuf, VendorId, PCI_ID_AS_STR_LENGTH) == 0)
{
Fix memory-safety and robustness issues in script engine and PCI ID parser Code audit of the script engine's scanner/token handling and of the PCI ID database parser. Each of the issues below was reproduced against the current code before the fix and re-checked afterwards. script-engine/scanner.c * An unterminated string literal ("abc or L"abc) hung the scanner in an endless loop: sgetc() returns EOF without consuming input, and neither string loop tested for it, so the token grew until allocation failed. Both loops now stop at EOF and report the token as UNKNOWN. script-engine/common.c * AppendByte()/AppendWchar() doubled Token->MaxLen before checking whether the larger buffer was actually allocated. After a failed allocation MaxLen described memory that did not exist and the next append wrote past the end of the old buffer. MaxLen is now committed only on success. * CopyToken() allocated strlen(Value) + 1 bytes but carried over the source token's Len and MaxLen, so the copy's advertised capacity did not match its allocation, and WSTRING payloads were truncated at their first embedded null byte. The copy is now sized from Len/MaxLen and copied by length, with a fallback to the string length for the grammar tokens in parse-table.c, which only initialize Type and Value. * NewToken() set MaxLen to the value length, which is zero for an empty value. The 'Len >= MaxLen - 1' test in the append routines is unsigned, so a zero MaxLen wrapped and disabled buffer growth entirely. * IsUnderscore() tested 'c >= '_'', which also accepted the backtick, the lowercase letters, '{', '|', '}', '~' and DEL. Register scanning uses it, so '@rax|1' was lexed as one malformed register name instead of a register, an operator and a number. The pseudo-register path already compared against '_' directly. * NewTokenList() did not check the allocation of its Head buffer. * NewTemp() kept the last handed-out id in a static, so an exhausted temp list produced a token aliasing a temporary still in use, and it derived MaxTempNumber from an out-of-range index. It also dereferenced the new token without a null check. * FreeTemp() indexed the MAX_TEMP_COUNT-entry map with an unchecked value parsed out of the token text. * RotateLeftStringOnce() wrote to str[-1] when handed an empty string. libhyperdbg/debugger/misc/pci-id.cpp * The database file was read into a malloc(Length) buffer that was never null-terminated, while ReadLine() walks it with strchr(). Looking up an absent vendor scans to the end and reads past the allocation. * The matched Vendor was allocated with malloc() and its Devices list head was only assigned once a device line was parsed, so a vendor with no device entries left it uninitialized and FreeVendor() walked a garbage pointer. * FreeVendor() released the device and subdevice lists but never the Vendor itself, leaking one per lookup for every enumerated PCI device. * The file handle leaked when the buffer allocation failed, ftell() and fread() results were unused, and several error paths leaked the Vendor or the not-yet-linked Device/SubDevice. * strncmp() compared sizeof(VendorId) bytes, which is the size of the pointer rather than the length of a vendor id. * ReadLine() passed an unclamped count to strncpy_s(), which triggers the invalid parameter handler for a line longer than the destination. * GetVendorById() ignored the GetModuleFileName() result and overwrote the tail of the path buffer without checking the room left in it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01W3C1DuhHtqK64eEkHjKCHM
2026-08-01 14:22:52 +00:00
//
// calloc() so that the Devices list head starts out empty: it is
// only assigned once a device line is parsed, and FreeVendor()
// would otherwise walk an uninitialized pointer for a vendor that
// has no devices listed
//
MatchedVendor = (Vendor *)calloc(1, sizeof(Vendor));
if (!MatchedVendor)
{
return NULL;
}
INT Result = sscanf(VendorBuf, "%hx", &(MatchedVendor->VendorId));
if (Result != 1)
{
Fix memory-safety and robustness issues in script engine and PCI ID parser Code audit of the script engine's scanner/token handling and of the PCI ID database parser. Each of the issues below was reproduced against the current code before the fix and re-checked afterwards. script-engine/scanner.c * An unterminated string literal ("abc or L"abc) hung the scanner in an endless loop: sgetc() returns EOF without consuming input, and neither string loop tested for it, so the token grew until allocation failed. Both loops now stop at EOF and report the token as UNKNOWN. script-engine/common.c * AppendByte()/AppendWchar() doubled Token->MaxLen before checking whether the larger buffer was actually allocated. After a failed allocation MaxLen described memory that did not exist and the next append wrote past the end of the old buffer. MaxLen is now committed only on success. * CopyToken() allocated strlen(Value) + 1 bytes but carried over the source token's Len and MaxLen, so the copy's advertised capacity did not match its allocation, and WSTRING payloads were truncated at their first embedded null byte. The copy is now sized from Len/MaxLen and copied by length, with a fallback to the string length for the grammar tokens in parse-table.c, which only initialize Type and Value. * NewToken() set MaxLen to the value length, which is zero for an empty value. The 'Len >= MaxLen - 1' test in the append routines is unsigned, so a zero MaxLen wrapped and disabled buffer growth entirely. * IsUnderscore() tested 'c >= '_'', which also accepted the backtick, the lowercase letters, '{', '|', '}', '~' and DEL. Register scanning uses it, so '@rax|1' was lexed as one malformed register name instead of a register, an operator and a number. The pseudo-register path already compared against '_' directly. * NewTokenList() did not check the allocation of its Head buffer. * NewTemp() kept the last handed-out id in a static, so an exhausted temp list produced a token aliasing a temporary still in use, and it derived MaxTempNumber from an out-of-range index. It also dereferenced the new token without a null check. * FreeTemp() indexed the MAX_TEMP_COUNT-entry map with an unchecked value parsed out of the token text. * RotateLeftStringOnce() wrote to str[-1] when handed an empty string. libhyperdbg/debugger/misc/pci-id.cpp * The database file was read into a malloc(Length) buffer that was never null-terminated, while ReadLine() walks it with strchr(). Looking up an absent vendor scans to the end and reads past the allocation. * The matched Vendor was allocated with malloc() and its Devices list head was only assigned once a device line was parsed, so a vendor with no device entries left it uninitialized and FreeVendor() walked a garbage pointer. * FreeVendor() released the device and subdevice lists but never the Vendor itself, leaking one per lookup for every enumerated PCI device. * The file handle leaked when the buffer allocation failed, ftell() and fread() results were unused, and several error paths leaked the Vendor or the not-yet-linked Device/SubDevice. * strncmp() compared sizeof(VendorId) bytes, which is the size of the pointer rather than the length of a vendor id. * ReadLine() passed an unclamped count to strncpy_s(), which triggers the invalid parameter handler for a line longer than the destination. * GetVendorById() ignored the GetModuleFileName() result and overwrote the tail of the path buffer without checking the room left in it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01W3C1DuhHtqK64eEkHjKCHM
2026-08-01 14:22:52 +00:00
FreeVendor(MatchedVendor);
return NULL;
}
PlatformStrNCpy(MatchedVendor->VendorName, sizeof(MatchedVendor->VendorName), TrimWhitespace(VendorNameBuf, PCI_NAME_STR_LENGTH), _TRUNCATE);
FoundVendorId = TRUE;
}
}
}
// Get all devices for vendor
else if (Line[0] == '\t' && Line[1] != '\t' && FoundVendorId == TRUE)
{
CHAR DeviceBuf[PCI_ID_AS_STR_LENGTH + 1], DeviceNameBuf[PCI_NAME_STR_LENGTH + 1];
snprintf(FormatStr, sizeof(FormatStr), "%%4s %%%d[^\n]", PCI_NAME_STR_LENGTH); // FormatStr = "%4s %PCI_NAME_STR_LENGTH[^\n]"
if (sscanf(Line + 1, FormatStr, DeviceBuf, DeviceNameBuf) == 2)
{
Device * NewDevice = (Device *)malloc(sizeof(Device));
if (!NewDevice)
{
FreeVendor(MatchedVendor);
return NULL;
}
int Result = sscanf(DeviceBuf, "%hx", &(NewDevice->DeviceId));
if (Result != 1)
{
Fix memory-safety and robustness issues in script engine and PCI ID parser Code audit of the script engine's scanner/token handling and of the PCI ID database parser. Each of the issues below was reproduced against the current code before the fix and re-checked afterwards. script-engine/scanner.c * An unterminated string literal ("abc or L"abc) hung the scanner in an endless loop: sgetc() returns EOF without consuming input, and neither string loop tested for it, so the token grew until allocation failed. Both loops now stop at EOF and report the token as UNKNOWN. script-engine/common.c * AppendByte()/AppendWchar() doubled Token->MaxLen before checking whether the larger buffer was actually allocated. After a failed allocation MaxLen described memory that did not exist and the next append wrote past the end of the old buffer. MaxLen is now committed only on success. * CopyToken() allocated strlen(Value) + 1 bytes but carried over the source token's Len and MaxLen, so the copy's advertised capacity did not match its allocation, and WSTRING payloads were truncated at their first embedded null byte. The copy is now sized from Len/MaxLen and copied by length, with a fallback to the string length for the grammar tokens in parse-table.c, which only initialize Type and Value. * NewToken() set MaxLen to the value length, which is zero for an empty value. The 'Len >= MaxLen - 1' test in the append routines is unsigned, so a zero MaxLen wrapped and disabled buffer growth entirely. * IsUnderscore() tested 'c >= '_'', which also accepted the backtick, the lowercase letters, '{', '|', '}', '~' and DEL. Register scanning uses it, so '@rax|1' was lexed as one malformed register name instead of a register, an operator and a number. The pseudo-register path already compared against '_' directly. * NewTokenList() did not check the allocation of its Head buffer. * NewTemp() kept the last handed-out id in a static, so an exhausted temp list produced a token aliasing a temporary still in use, and it derived MaxTempNumber from an out-of-range index. It also dereferenced the new token without a null check. * FreeTemp() indexed the MAX_TEMP_COUNT-entry map with an unchecked value parsed out of the token text. * RotateLeftStringOnce() wrote to str[-1] when handed an empty string. libhyperdbg/debugger/misc/pci-id.cpp * The database file was read into a malloc(Length) buffer that was never null-terminated, while ReadLine() walks it with strchr(). Looking up an absent vendor scans to the end and reads past the allocation. * The matched Vendor was allocated with malloc() and its Devices list head was only assigned once a device line was parsed, so a vendor with no device entries left it uninitialized and FreeVendor() walked a garbage pointer. * FreeVendor() released the device and subdevice lists but never the Vendor itself, leaking one per lookup for every enumerated PCI device. * The file handle leaked when the buffer allocation failed, ftell() and fread() results were unused, and several error paths leaked the Vendor or the not-yet-linked Device/SubDevice. * strncmp() compared sizeof(VendorId) bytes, which is the size of the pointer rather than the length of a vendor id. * ReadLine() passed an unclamped count to strncpy_s(), which triggers the invalid parameter handler for a line longer than the destination. * GetVendorById() ignored the GetModuleFileName() result and overwrote the tail of the path buffer without checking the room left in it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01W3C1DuhHtqK64eEkHjKCHM
2026-08-01 14:22:52 +00:00
//
// NewDevice is not linked into the vendor's list yet, so it has to
// be released separately from FreeVendor()
//
free(NewDevice);
FreeVendor(MatchedVendor);
return NULL;
}
PlatformStrNCpy(NewDevice->DeviceName, sizeof(NewDevice->DeviceName), TrimWhitespace(DeviceNameBuf, PCI_NAME_STR_LENGTH), _TRUNCATE);
NewDevice->SubDevices = NULL;
NewDevice->Next = NULL;
if (LastDevice)
{
LastDevice->Next = NewDevice;
}
else
{
MatchedVendor->Devices = NewDevice; // First device
}
LastDevice = NewDevice;
LastSubDevice = NULL;
}
}
// Get all subdevices for device
else if (Line[0] == '\t' && Line[1] == '\t' && FoundVendorId == TRUE && LastDevice)
{
CHAR SubVendorBuf[PCI_ID_AS_STR_LENGTH + 1], SubDeviceBuf[PCI_ID_AS_STR_LENGTH + 1], SubsystemNameBuf[PCI_NAME_STR_LENGTH + 1];
snprintf(FormatStr, sizeof(FormatStr), "%%4s %%4s %%%d[^\n]", PCI_NAME_STR_LENGTH); // FormatStr = "%4s %4s %PCI_NAME_STR_LENGTH[^\n]"
if (sscanf(Line + 2, FormatStr, SubVendorBuf, SubDeviceBuf, SubsystemNameBuf) == 3)
{
SubDevice * NewSubDevice = (SubDevice *)malloc(sizeof(SubDevice));
if (!NewSubDevice)
{
FreeVendor(MatchedVendor);
return NULL;
}
int Result = sscanf(SubVendorBuf, "%hx", &NewSubDevice->SubVendorId);
if (Result != 1)
{
Fix memory-safety and robustness issues in script engine and PCI ID parser Code audit of the script engine's scanner/token handling and of the PCI ID database parser. Each of the issues below was reproduced against the current code before the fix and re-checked afterwards. script-engine/scanner.c * An unterminated string literal ("abc or L"abc) hung the scanner in an endless loop: sgetc() returns EOF without consuming input, and neither string loop tested for it, so the token grew until allocation failed. Both loops now stop at EOF and report the token as UNKNOWN. script-engine/common.c * AppendByte()/AppendWchar() doubled Token->MaxLen before checking whether the larger buffer was actually allocated. After a failed allocation MaxLen described memory that did not exist and the next append wrote past the end of the old buffer. MaxLen is now committed only on success. * CopyToken() allocated strlen(Value) + 1 bytes but carried over the source token's Len and MaxLen, so the copy's advertised capacity did not match its allocation, and WSTRING payloads were truncated at their first embedded null byte. The copy is now sized from Len/MaxLen and copied by length, with a fallback to the string length for the grammar tokens in parse-table.c, which only initialize Type and Value. * NewToken() set MaxLen to the value length, which is zero for an empty value. The 'Len >= MaxLen - 1' test in the append routines is unsigned, so a zero MaxLen wrapped and disabled buffer growth entirely. * IsUnderscore() tested 'c >= '_'', which also accepted the backtick, the lowercase letters, '{', '|', '}', '~' and DEL. Register scanning uses it, so '@rax|1' was lexed as one malformed register name instead of a register, an operator and a number. The pseudo-register path already compared against '_' directly. * NewTokenList() did not check the allocation of its Head buffer. * NewTemp() kept the last handed-out id in a static, so an exhausted temp list produced a token aliasing a temporary still in use, and it derived MaxTempNumber from an out-of-range index. It also dereferenced the new token without a null check. * FreeTemp() indexed the MAX_TEMP_COUNT-entry map with an unchecked value parsed out of the token text. * RotateLeftStringOnce() wrote to str[-1] when handed an empty string. libhyperdbg/debugger/misc/pci-id.cpp * The database file was read into a malloc(Length) buffer that was never null-terminated, while ReadLine() walks it with strchr(). Looking up an absent vendor scans to the end and reads past the allocation. * The matched Vendor was allocated with malloc() and its Devices list head was only assigned once a device line was parsed, so a vendor with no device entries left it uninitialized and FreeVendor() walked a garbage pointer. * FreeVendor() released the device and subdevice lists but never the Vendor itself, leaking one per lookup for every enumerated PCI device. * The file handle leaked when the buffer allocation failed, ftell() and fread() results were unused, and several error paths leaked the Vendor or the not-yet-linked Device/SubDevice. * strncmp() compared sizeof(VendorId) bytes, which is the size of the pointer rather than the length of a vendor id. * ReadLine() passed an unclamped count to strncpy_s(), which triggers the invalid parameter handler for a line longer than the destination. * GetVendorById() ignored the GetModuleFileName() result and overwrote the tail of the path buffer without checking the room left in it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01W3C1DuhHtqK64eEkHjKCHM
2026-08-01 14:22:52 +00:00
//
// NewSubDevice is not linked into the device's list yet, so it has
// to be released separately from FreeVendor()
//
free(NewSubDevice);
FreeVendor(MatchedVendor);
return NULL;
}
Result = sscanf(SubDeviceBuf, "%hx", &NewSubDevice->SubDeviceId);
if (Result != 1)
{
Fix memory-safety and robustness issues in script engine and PCI ID parser Code audit of the script engine's scanner/token handling and of the PCI ID database parser. Each of the issues below was reproduced against the current code before the fix and re-checked afterwards. script-engine/scanner.c * An unterminated string literal ("abc or L"abc) hung the scanner in an endless loop: sgetc() returns EOF without consuming input, and neither string loop tested for it, so the token grew until allocation failed. Both loops now stop at EOF and report the token as UNKNOWN. script-engine/common.c * AppendByte()/AppendWchar() doubled Token->MaxLen before checking whether the larger buffer was actually allocated. After a failed allocation MaxLen described memory that did not exist and the next append wrote past the end of the old buffer. MaxLen is now committed only on success. * CopyToken() allocated strlen(Value) + 1 bytes but carried over the source token's Len and MaxLen, so the copy's advertised capacity did not match its allocation, and WSTRING payloads were truncated at their first embedded null byte. The copy is now sized from Len/MaxLen and copied by length, with a fallback to the string length for the grammar tokens in parse-table.c, which only initialize Type and Value. * NewToken() set MaxLen to the value length, which is zero for an empty value. The 'Len >= MaxLen - 1' test in the append routines is unsigned, so a zero MaxLen wrapped and disabled buffer growth entirely. * IsUnderscore() tested 'c >= '_'', which also accepted the backtick, the lowercase letters, '{', '|', '}', '~' and DEL. Register scanning uses it, so '@rax|1' was lexed as one malformed register name instead of a register, an operator and a number. The pseudo-register path already compared against '_' directly. * NewTokenList() did not check the allocation of its Head buffer. * NewTemp() kept the last handed-out id in a static, so an exhausted temp list produced a token aliasing a temporary still in use, and it derived MaxTempNumber from an out-of-range index. It also dereferenced the new token without a null check. * FreeTemp() indexed the MAX_TEMP_COUNT-entry map with an unchecked value parsed out of the token text. * RotateLeftStringOnce() wrote to str[-1] when handed an empty string. libhyperdbg/debugger/misc/pci-id.cpp * The database file was read into a malloc(Length) buffer that was never null-terminated, while ReadLine() walks it with strchr(). Looking up an absent vendor scans to the end and reads past the allocation. * The matched Vendor was allocated with malloc() and its Devices list head was only assigned once a device line was parsed, so a vendor with no device entries left it uninitialized and FreeVendor() walked a garbage pointer. * FreeVendor() released the device and subdevice lists but never the Vendor itself, leaking one per lookup for every enumerated PCI device. * The file handle leaked when the buffer allocation failed, ftell() and fread() results were unused, and several error paths leaked the Vendor or the not-yet-linked Device/SubDevice. * strncmp() compared sizeof(VendorId) bytes, which is the size of the pointer rather than the length of a vendor id. * ReadLine() passed an unclamped count to strncpy_s(), which triggers the invalid parameter handler for a line longer than the destination. * GetVendorById() ignored the GetModuleFileName() result and overwrote the tail of the path buffer without checking the room left in it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01W3C1DuhHtqK64eEkHjKCHM
2026-08-01 14:22:52 +00:00
free(NewSubDevice);
FreeVendor(MatchedVendor);
return NULL;
}
PlatformStrNCpy(NewSubDevice->SubSystemName, sizeof(NewSubDevice->SubSystemName), TrimWhitespace(SubsystemNameBuf, PCI_NAME_STR_LENGTH), _TRUNCATE);
NewSubDevice->Next = NULL;
if (LastSubDevice)
{
LastSubDevice->Next = NewSubDevice;
}
else
{
LastDevice->SubDevices = NewSubDevice; // First subdevice
}
LastSubDevice = NewSubDevice;
}
}
else if (Line[0] != '\t' && FoundVendorId == TRUE) // We hit the next vendor entry, so we're done parsing
{
break;
}
}
return MatchedVendor;
}
/**
* @brief Frees Vendor and all of its members
*
* @param VendorToFree
* @return VOID
*/
VOID
FreeVendor(Vendor * VendorToFree)
{
if (VendorToFree == NULL)
return;
Device * CurrentDevice = VendorToFree->Devices;
while (CurrentDevice)
{
SubDevice * CurrentSubDevice = CurrentDevice->SubDevices;
while (CurrentSubDevice)
{
SubDevice * NextSubDevice = CurrentSubDevice->Next;
free(CurrentSubDevice);
CurrentSubDevice = NextSubDevice;
}
Device * NextDevice = CurrentDevice->Next;
free(CurrentDevice);
CurrentDevice = NextDevice;
}
Fix memory-safety and robustness issues in script engine and PCI ID parser Code audit of the script engine's scanner/token handling and of the PCI ID database parser. Each of the issues below was reproduced against the current code before the fix and re-checked afterwards. script-engine/scanner.c * An unterminated string literal ("abc or L"abc) hung the scanner in an endless loop: sgetc() returns EOF without consuming input, and neither string loop tested for it, so the token grew until allocation failed. Both loops now stop at EOF and report the token as UNKNOWN. script-engine/common.c * AppendByte()/AppendWchar() doubled Token->MaxLen before checking whether the larger buffer was actually allocated. After a failed allocation MaxLen described memory that did not exist and the next append wrote past the end of the old buffer. MaxLen is now committed only on success. * CopyToken() allocated strlen(Value) + 1 bytes but carried over the source token's Len and MaxLen, so the copy's advertised capacity did not match its allocation, and WSTRING payloads were truncated at their first embedded null byte. The copy is now sized from Len/MaxLen and copied by length, with a fallback to the string length for the grammar tokens in parse-table.c, which only initialize Type and Value. * NewToken() set MaxLen to the value length, which is zero for an empty value. The 'Len >= MaxLen - 1' test in the append routines is unsigned, so a zero MaxLen wrapped and disabled buffer growth entirely. * IsUnderscore() tested 'c >= '_'', which also accepted the backtick, the lowercase letters, '{', '|', '}', '~' and DEL. Register scanning uses it, so '@rax|1' was lexed as one malformed register name instead of a register, an operator and a number. The pseudo-register path already compared against '_' directly. * NewTokenList() did not check the allocation of its Head buffer. * NewTemp() kept the last handed-out id in a static, so an exhausted temp list produced a token aliasing a temporary still in use, and it derived MaxTempNumber from an out-of-range index. It also dereferenced the new token without a null check. * FreeTemp() indexed the MAX_TEMP_COUNT-entry map with an unchecked value parsed out of the token text. * RotateLeftStringOnce() wrote to str[-1] when handed an empty string. libhyperdbg/debugger/misc/pci-id.cpp * The database file was read into a malloc(Length) buffer that was never null-terminated, while ReadLine() walks it with strchr(). Looking up an absent vendor scans to the end and reads past the allocation. * The matched Vendor was allocated with malloc() and its Devices list head was only assigned once a device line was parsed, so a vendor with no device entries left it uninitialized and FreeVendor() walked a garbage pointer. * FreeVendor() released the device and subdevice lists but never the Vendor itself, leaking one per lookup for every enumerated PCI device. * The file handle leaked when the buffer allocation failed, ftell() and fread() results were unused, and several error paths leaked the Vendor or the not-yet-linked Device/SubDevice. * strncmp() compared sizeof(VendorId) bytes, which is the size of the pointer rather than the length of a vendor id. * ReadLine() passed an unclamped count to strncpy_s(), which triggers the invalid parameter handler for a line longer than the destination. * GetVendorById() ignored the GetModuleFileName() result and overwrote the tail of the path buffer without checking the room left in it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01W3C1DuhHtqK64eEkHjKCHM
2026-08-01 14:22:52 +00:00
//
// The Vendor itself is allocated by GetVendorByIdStr() and was previously never
// released, leaking one Vendor per call for every PCI device that got enumerated
//
VendorToFree->Devices = NULL;
free(VendorToFree);
}
/**
* @brief Frees PciIdDatabaseBuffer
* @return VOID
*/
VOID
FreePciIdDatabase()
{
if (PciIdDatabaseBuffer != NULL)
{
free(PciIdDatabaseBuffer);
PciIdDatabaseBuffer = NULL;
}
}
/**
* @brief Returns Vendor entry, including corresponding devices and subdevices
* @details Use FreeVendor() on returned Vendor pointer after usage. First call will initialize database - call FreeDatabase() once done querying.
*
* @param VendorId
* @return Vendor
*/
Vendor *
GetVendorById(UINT16 VendorId)
{
#ifdef _WIN32
CHAR VendorIdAsStr[5];
CHAR ExecutablePath[MAX_PATH];
HMODULE hModule = GetModuleHandle(NULL);
snprintf(VendorIdAsStr, sizeof(VendorIdAsStr), "%04X", VendorId);
Fix memory-safety and robustness issues in script engine and PCI ID parser Code audit of the script engine's scanner/token handling and of the PCI ID database parser. Each of the issues below was reproduced against the current code before the fix and re-checked afterwards. script-engine/scanner.c * An unterminated string literal ("abc or L"abc) hung the scanner in an endless loop: sgetc() returns EOF without consuming input, and neither string loop tested for it, so the token grew until allocation failed. Both loops now stop at EOF and report the token as UNKNOWN. script-engine/common.c * AppendByte()/AppendWchar() doubled Token->MaxLen before checking whether the larger buffer was actually allocated. After a failed allocation MaxLen described memory that did not exist and the next append wrote past the end of the old buffer. MaxLen is now committed only on success. * CopyToken() allocated strlen(Value) + 1 bytes but carried over the source token's Len and MaxLen, so the copy's advertised capacity did not match its allocation, and WSTRING payloads were truncated at their first embedded null byte. The copy is now sized from Len/MaxLen and copied by length, with a fallback to the string length for the grammar tokens in parse-table.c, which only initialize Type and Value. * NewToken() set MaxLen to the value length, which is zero for an empty value. The 'Len >= MaxLen - 1' test in the append routines is unsigned, so a zero MaxLen wrapped and disabled buffer growth entirely. * IsUnderscore() tested 'c >= '_'', which also accepted the backtick, the lowercase letters, '{', '|', '}', '~' and DEL. Register scanning uses it, so '@rax|1' was lexed as one malformed register name instead of a register, an operator and a number. The pseudo-register path already compared against '_' directly. * NewTokenList() did not check the allocation of its Head buffer. * NewTemp() kept the last handed-out id in a static, so an exhausted temp list produced a token aliasing a temporary still in use, and it derived MaxTempNumber from an out-of-range index. It also dereferenced the new token without a null check. * FreeTemp() indexed the MAX_TEMP_COUNT-entry map with an unchecked value parsed out of the token text. * RotateLeftStringOnce() wrote to str[-1] when handed an empty string. libhyperdbg/debugger/misc/pci-id.cpp * The database file was read into a malloc(Length) buffer that was never null-terminated, while ReadLine() walks it with strchr(). Looking up an absent vendor scans to the end and reads past the allocation. * The matched Vendor was allocated with malloc() and its Devices list head was only assigned once a device line was parsed, so a vendor with no device entries left it uninitialized and FreeVendor() walked a garbage pointer. * FreeVendor() released the device and subdevice lists but never the Vendor itself, leaking one per lookup for every enumerated PCI device. * The file handle leaked when the buffer allocation failed, ftell() and fread() results were unused, and several error paths leaked the Vendor or the not-yet-linked Device/SubDevice. * strncmp() compared sizeof(VendorId) bytes, which is the size of the pointer rather than the length of a vendor id. * ReadLine() passed an unclamped count to strncpy_s(), which triggers the invalid parameter handler for a line longer than the destination. * GetVendorById() ignored the GetModuleFileName() result and overwrote the tail of the path buffer without checking the room left in it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01W3C1DuhHtqK64eEkHjKCHM
2026-08-01 14:22:52 +00:00
DWORD PathLength = GetModuleFileName(hModule, ExecutablePath, sizeof(ExecutablePath));
//
// A zero length means the call failed; a length equal to the buffer size means the
// path was truncated and, on older Windows versions, left without a null terminator
//
if (PathLength == 0 || PathLength >= sizeof(ExecutablePath))
{
return NULL;
}
// Extract executable name
CHAR * ExecutableName = strrchr(ExecutablePath, '\\');
if (ExecutableName != NULL)
{
ExecutableName++;
}
else
{
ExecutableName = ExecutablePath;
}
// Swap executable name for PCI_ID_DATABASE_PATH
Fix memory-safety and robustness issues in script engine and PCI ID parser Code audit of the script engine's scanner/token handling and of the PCI ID database parser. Each of the issues below was reproduced against the current code before the fix and re-checked afterwards. script-engine/scanner.c * An unterminated string literal ("abc or L"abc) hung the scanner in an endless loop: sgetc() returns EOF without consuming input, and neither string loop tested for it, so the token grew until allocation failed. Both loops now stop at EOF and report the token as UNKNOWN. script-engine/common.c * AppendByte()/AppendWchar() doubled Token->MaxLen before checking whether the larger buffer was actually allocated. After a failed allocation MaxLen described memory that did not exist and the next append wrote past the end of the old buffer. MaxLen is now committed only on success. * CopyToken() allocated strlen(Value) + 1 bytes but carried over the source token's Len and MaxLen, so the copy's advertised capacity did not match its allocation, and WSTRING payloads were truncated at their first embedded null byte. The copy is now sized from Len/MaxLen and copied by length, with a fallback to the string length for the grammar tokens in parse-table.c, which only initialize Type and Value. * NewToken() set MaxLen to the value length, which is zero for an empty value. The 'Len >= MaxLen - 1' test in the append routines is unsigned, so a zero MaxLen wrapped and disabled buffer growth entirely. * IsUnderscore() tested 'c >= '_'', which also accepted the backtick, the lowercase letters, '{', '|', '}', '~' and DEL. Register scanning uses it, so '@rax|1' was lexed as one malformed register name instead of a register, an operator and a number. The pseudo-register path already compared against '_' directly. * NewTokenList() did not check the allocation of its Head buffer. * NewTemp() kept the last handed-out id in a static, so an exhausted temp list produced a token aliasing a temporary still in use, and it derived MaxTempNumber from an out-of-range index. It also dereferenced the new token without a null check. * FreeTemp() indexed the MAX_TEMP_COUNT-entry map with an unchecked value parsed out of the token text. * RotateLeftStringOnce() wrote to str[-1] when handed an empty string. libhyperdbg/debugger/misc/pci-id.cpp * The database file was read into a malloc(Length) buffer that was never null-terminated, while ReadLine() walks it with strchr(). Looking up an absent vendor scans to the end and reads past the allocation. * The matched Vendor was allocated with malloc() and its Devices list head was only assigned once a device line was parsed, so a vendor with no device entries left it uninitialized and FreeVendor() walked a garbage pointer. * FreeVendor() released the device and subdevice lists but never the Vendor itself, leaking one per lookup for every enumerated PCI device. * The file handle leaked when the buffer allocation failed, ftell() and fread() results were unused, and several error paths leaked the Vendor or the not-yet-linked Device/SubDevice. * strncmp() compared sizeof(VendorId) bytes, which is the size of the pointer rather than the length of a vendor id. * ReadLine() passed an unclamped count to strncpy_s(), which triggers the invalid parameter handler for a line longer than the destination. * GetVendorById() ignored the GetModuleFileName() result and overwrote the tail of the path buffer without checking the room left in it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01W3C1DuhHtqK64eEkHjKCHM
2026-08-01 14:22:52 +00:00
//
// The database path can be longer than the executable name it replaces, so the
// room left in ExecutablePath is checked before overwriting the tail
//
SIZE_T RemainingSpace = sizeof(ExecutablePath) - (SIZE_T)(ExecutableName - ExecutablePath);
if (RemainingSpace < sizeof(PCI_ID_DATABASE_PATH))
{
return NULL;
}
memcpy(ExecutableName, PCI_ID_DATABASE_PATH, sizeof(PCI_ID_DATABASE_PATH));
return GetVendorByIdStr(ExecutablePath, ToLower(VendorIdAsStr));
#else
//
// TODO(Linux): resolve the PCI ID database next to the executable via
// readlink("/proc/self/exe") once the path separator and
// PCI_ID_DATABASE_PATH ("constants\\pci.ids") are made portable. Until
// then no vendor/device names are available on Linux.
//
UNREFERENCED_PARAMETER(VendorId);
return NULL;
#endif
}
/**
* @brief Returns Device entry corresponding to DeviceId
*
* @param VendorToUse
* @param DeviceId
* @return Device
*/
Device *
GetDeviceFromVendor(Vendor * VendorToUse, UINT16 DeviceId)
{
Device * CurrentDevice = NULL;
if (!VendorToUse)
{
return NULL;
}
CurrentDevice = VendorToUse->Devices;
while (CurrentDevice != NULL)
{
if (CurrentDevice->DeviceId == DeviceId)
{
return CurrentDevice;
}
CurrentDevice = CurrentDevice->Next;
}
return NULL;
}
/**
* @brief Returns SubDevice entry corresponding to SubVendorId and DeviceId
*
* @param DeviceToUse
* @param SubVendorId
* @param SubDeviceId
* @return SubDevice
*/
SubDevice *
GetSubDeviceFromDevice(Device * DeviceToUse, UINT16 SubVendorId, UINT16 SubDeviceId)
{
SubDevice * CurrentSubDevice = NULL;
if (!DeviceToUse)
{
return NULL;
}
CurrentSubDevice = DeviceToUse->SubDevices;
while (CurrentSubDevice != NULL)
{
if (CurrentSubDevice->SubVendorId == SubVendorId && CurrentSubDevice->SubDeviceId == SubDeviceId)
{
return CurrentSubDevice;
}
CurrentSubDevice = CurrentSubDevice->Next;
}
return NULL;
}