apache / trafficserver

Apache Traffic Serverâ„¢ is a fast, scalable and extensible HTTP/1.1 and HTTP/2 compliant caching proxy server.
https://trafficserver.apache.org/
Apache License 2.0
1.74k stars 781 forks source link

Fix use of uninitialized stack memory in records #11450

Closed JosiahWI closed 1 week ago

JosiahWI commented 2 weeks ago

Fixes #11449.

This adds an overload of RecGetRecordString that takes an out-parameter for the size of the content copied into the out buffer. It updates RecHTTPLoadIpAddrsFromConfVars to use this parameter to get the buffer size to use when initializing the TextView for parsing, so that we only parse the actual content and ignore the uninitialized part of the buffer.

This passes the length of the value read to the TextView constructor so that it only parses the actual content and ignores the uninitialized part of the buffer.

JosiahWI commented 2 weeks ago

This fix is incorrect and introduces a buffer overflow vulnerability because strlcpy returns the total length of the source string, not the destination string.

JosiahWI commented 2 weeks ago

Should be good now.

ywkaras commented 1 week ago

Both GCC and CLang seem to be very good at evaluation strlen for string constants at runtime: https://godbolt.org/z/fqnd5Pb4n . So, we should change TextView and remove the complicated attempts to avoid calling strlen.

ywkaras commented 1 week ago

The template overload for C string literals doesn't seem to work anyway: https://godbolt.org/z/5cTWxYq9v