Skip to content

Commit

Permalink
Fix out-of-bounds read in ChemistryStr
Browse files Browse the repository at this point in the history
- 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
  • Loading branch information
jay committed Apr 26, 2022
1 parent 0bdd8a1 commit ad6e095
Showing 1 changed file with 7 additions and 1 deletion.
8 changes: 7 additions & 1 deletion battstatus.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down

0 comments on commit ad6e095

Please sign in to comment.