search

ros-perception / image_transport_plugins

A set of plugins for publishing and subscribing to sensor_msgs/Image topics in representations other than raw pixel data.
BSD 3-Clause "New" or "Revised" License
58 stars 119 forks source link

Fix uninitialized memory usage in compressedDepth transport #125

Closed peci1 closed 1 year ago

peci1 commented 1 year ago

The ConfigHeader struct in compressedDepth encoder was not initialized (or at least zero-initialized by the compiler). So compressionConfig.depthParam array contains uninitialized memory. For 32-bit images, that's fine because these values are overwritten later. However, for 16UC1, there are no writes to these variables, meaning the uninitilized memory content leaks to the output compressed messages! So this might even be a security issue (although I have no POC).

This simple change will zero-initialize the struct so that the variables are zero by default.

I don't think there could be any bad consequences for any downstream code. The values are "random" now, so nobody could rely on their contents in the 16UC1 case. And 32FC1 behavior remains untouched.

Here's an illustrative memory view of a message I've just loaded from a bag file I had: in the upper part, you can see the ConfigHeader extracted from the data, and in the lower part, you can see the bytes. Normally, all bytes preceding \211PNG should be zero. With this PR, they are.

image

ijnek commented 1 year ago

Thanks @peci1 !

ijnek commented 1 year ago

@Mergifyio backport melodic-devel

mergify[bot] commented 1 year ago

backport melodic-devel

✅ Backports have been created

* [#126 Fix uninitialized memory usage in compressedDepth transport (backport #125)](https://github.com/ros-perception/image_transport_plugins/pull/126) has been created for branch `melodic-devel`