opencomputeproject / SAI

Switch Abstraction Interface
Other
472 stars 475 forks source link

buffer overflow vulnerability in sai_serialize_attribute function #1887

Open rameshms-work opened 1 year ago

rameshms-work commented 1 year ago

sai_serialize_attribute function (and other functions it calls) doesn't take into consideration the output buffer size. This can be a real issue when serializing variable lenght attributes such as obj_list/u32list etc. Ideally this function should have argument "int buflen" and use API's that take into account the buffer length such as snprintf().

kcudnik commented 1 year ago

yes, this is like sprintf function, currently this is no silimar function that will take a buffer size, you are wellcome to add that functionality, and as current workaround, i propose to declare a buffer with some size let say 12k, and set last page as non writable page, so you will have 8h buffer and 4k protector page, this way if there will be exception you can safely catch it and you know that this is a buffer overflow, and program can still execute

in sairedis library we are using serialization using std::string, so this problem don't exist, but libsaimetadata we want to be ansi C compatible , so we are are not planning to add c++ support here.

rameshms-work commented 1 year ago

I did a quick look at the code in sonic-sairedis. I'm not too familiar with SONiC /SAI-Redis code. From what I understand, it seems like the C++ wrapper (inside ClientSai.cpp) still uses the SAI implementation to serialize/unserialize the attributes.

kcudnik commented 1 year ago

sairedis is not using this code from SAI/meta, functions have the same naming but it uses cpp implementaion in sonic-sairedis/meta/SaiSerialize.cpp