From ad6e0951a6e809500a161a956c4bae44bd51b285 Mon Sep 17 00:00:00 2001 From: Jay Satiro Date: Tue, 26 Apr 2022 00:24:18 -0400 Subject: [PATCH] Fix out-of-bounds read in ChemistryStr - Check BATTERY_INFORMATION Chemistry[4] for null terminator. Chemistry is a string that is "not necessarily zero-terminated". Prior to this change ChemistryStr() did not check for null and incorrectly used sizeof to get the read size of parameter UCHAR Chemistry[4] which decays to UCHAR *Chemistry. For 32-bit builds ChemistryStr happened to work by luck, though it may have read the null into std::string. MS currently uses only strings of length 3 or 4, and the size of *Chemistry is 4 so ChemistryStr never read past 4 bytes. For example {'N','i','Z','n'} or {'R','A','M','\0'}. For 64-bit builds the pointer size is 8 and it read at least 4 bytes too many. Ref: https://docs.microsoft.com/en-us/windows/win32/power/battery-information-str --- battstatus.cpp | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/battstatus.cpp b/battstatus.cpp index 0c92d3f..1faf75f 100644 --- a/battstatus.cpp +++ b/battstatus.cpp @@ -309,7 +309,13 @@ string TechnologyStr(UCHAR Technology) string ChemistryStr(const UCHAR Chemistry[4]) { - return string((char *)Chemistry, sizeof Chemistry); + /* Chemistry is "not necessarily zero-terminated" */ + int i; + for(i = 0; i < 4; ++i) { + if(Chemistry[i] == '\0') + break; + } + return string((const char *)Chemistry, i); } string DesignedCapacityStr(ULONG DesignedCapacity,