-
Notifications
You must be signed in to change notification settings - Fork 5.3k
Description
I'm currently working on fuzz test(which generates random bytes for onData() and onWrite() to see whether we could crash the Envoy) for network-level filters.
When I was testing on postres_proxy filter with some untrusted data, an assert failure occurred inside linearize:
RELEASE_ASSERT(size <= length(), "Linearize size exceeds buffer size");
https://github.com/envoyproxy/envoy/blob/master/source/extensions/filters/network/postgres_proxy/postgres_decoder.cc#L201
This error only happens in fuzzer or when upstream server is on bad state, so it is not security-critical now.
But I think that we could deal with this error more gracefully. (So that we could make the filter more robust to upstream errors, and enable the fuzzer to continue testing it).
My idea is that we could make it just like other invalid error handles in this file, which is return false;, before calling linearize?
This solution looks like this(from line 200):
auto bytesToRead = length - 4;
if(bytesToRead>data.length()){
return false;
}
message.assign(std::string(static_cast<char*>(data.linearize(bytesToRead)), bytesToRead));
This issue can be reproduced in unit test by adding a case as below(test/extensions/filters/network/postgres_proxy/postgres_decoder_test.cc):
TEST_P(PostgresProxyFrontendEncrDecoderTest, AssertFailure) {
std::string str_data;
for(int i=0;i<8;i++){
str_data.push_back('\0');
}
Buffer::OwnedImpl data(str_data);
decoder_->onData(data, false);
}
If anyone has a better idea, please share with me or make a pull request and link it here. Thanks!
/cc @dio
/cc @fabriziomello
/cc @cpakulski