Olivier Lacoste-Bouchet opened issue #5: ipmi_cmd_get_device_sdr/get_sdr_data unchecked buffer over run at OpenIPMC / OpenIPMC-SW

Olivier Lacoste-Bouchet
2022-11-11T17:50:48Z

What version of the IPMI Spec is OpenIPMC using? Spec v2.0 specifies that 0xFF should be passed as byte 6 in a "get_device_sdr" request to "read the entire record". What this implies with the limited size of IPMI message at 32 -- I'm not sure. All I know is that the condition is not checked.

in ipmi_cmd_get_device_sdr, byte_to_read is unchecked until after calling into get_sdr_data which also does not check byte_to_read. bytes_to_read could not be a parameter in this function, but rather be set in the case, or just check it elsewhere (somewhere!!).

Could also be fixed by checking:

// Protections against querying more bytes than possible
if( bytes_to_read > 22 ) // If more than IPMI command limit
{
*res_len = 0;
*completion_code = 0xCA; // Can not return number of requested data bytes
return;
}

after assigning record_id, so we don't 1. possibly overwrite data beyond the buffer in get_sdr_data 2. do all that work just to return an invalid number of bytes anyways.

This is the loop that overwrites beyond what it should, line ~158, sdr_manager.c

//Load only the requested bytes into buffer
for (i=0; i<bytes_to_read; i++)
record_data_buff[i] = sdr_binary[offset_in_record + i];