mirror of
https://github.com/privatevoid-net/nix-super.git
synced 2024-11-15 02:36:16 +02:00
fix(libstore-tests): remove use-after-free bug for StringSource
Unfortunately `StringSource` class is very easy was very easy to misuse because the ctor took a plain `std::string_view` which has a bad habit of being implicitly convertible from an rvalue `std::string`. This lead to unintentional use-after-free bugs. This patch makes `StringSource` much harder to misuse by disabling the ctor from a `std::string &&` (but `const std::string &` is ok). Fix affected tests from libstore-tests. Reformat those tests with clangd's range formatting since the diff is tiny and it seems appropriate.
This commit is contained in:
parent
e65510da56
commit
5bc8957c73
3 changed files with 16 additions and 22 deletions
|
@ -459,21 +459,14 @@ TEST_F(ServeProtoTest, handshake_client_truncated_replay_throws)
|
||||||
CharacterizationTest::readTest("handshake-to-client", [&](std::string toClientLog) {
|
CharacterizationTest::readTest("handshake-to-client", [&](std::string toClientLog) {
|
||||||
for (size_t len = 0; len < toClientLog.size(); ++len) {
|
for (size_t len = 0; len < toClientLog.size(); ++len) {
|
||||||
NullBufferedSink nullSink;
|
NullBufferedSink nullSink;
|
||||||
StringSource in {
|
auto substring = toClientLog.substr(0, len);
|
||||||
// truncate
|
StringSource in{substring};
|
||||||
toClientLog.substr(0, len)
|
|
||||||
};
|
|
||||||
if (len < 8) {
|
if (len < 8) {
|
||||||
EXPECT_THROW(
|
EXPECT_THROW(
|
||||||
ServeProto::BasicClientConnection::handshake(
|
ServeProto::BasicClientConnection::handshake(nullSink, in, defaultVersion, "blah"), EndOfFile);
|
||||||
nullSink, in, defaultVersion, "blah"),
|
|
||||||
EndOfFile);
|
|
||||||
} else {
|
} else {
|
||||||
// Not sure why cannot keep on checking for `EndOfFile`.
|
// Not sure why cannot keep on checking for `EndOfFile`.
|
||||||
EXPECT_THROW(
|
EXPECT_THROW(ServeProto::BasicClientConnection::handshake(nullSink, in, defaultVersion, "blah"), Error);
|
||||||
ServeProto::BasicClientConnection::handshake(
|
|
||||||
nullSink, in, defaultVersion, "blah"),
|
|
||||||
Error);
|
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
});
|
});
|
||||||
|
|
|
@ -725,21 +725,14 @@ TEST_F(WorkerProtoTest, handshake_client_truncated_replay_throws)
|
||||||
CharacterizationTest::readTest("handshake-to-client", [&](std::string toClientLog) {
|
CharacterizationTest::readTest("handshake-to-client", [&](std::string toClientLog) {
|
||||||
for (size_t len = 0; len < toClientLog.size(); ++len) {
|
for (size_t len = 0; len < toClientLog.size(); ++len) {
|
||||||
NullBufferedSink nullSink;
|
NullBufferedSink nullSink;
|
||||||
StringSource in {
|
auto substring = toClientLog.substr(0, len);
|
||||||
// truncate
|
StringSource in{substring};
|
||||||
toClientLog.substr(0, len)
|
|
||||||
};
|
|
||||||
if (len < 8) {
|
if (len < 8) {
|
||||||
EXPECT_THROW(
|
EXPECT_THROW(
|
||||||
WorkerProto::BasicClientConnection::handshake(
|
WorkerProto::BasicClientConnection::handshake(nullSink, in, defaultVersion, {}), EndOfFile);
|
||||||
nullSink, in, defaultVersion, {}),
|
|
||||||
EndOfFile);
|
|
||||||
} else {
|
} else {
|
||||||
// Not sure why cannot keep on checking for `EndOfFile`.
|
// Not sure why cannot keep on checking for `EndOfFile`.
|
||||||
EXPECT_THROW(
|
EXPECT_THROW(WorkerProto::BasicClientConnection::handshake(nullSink, in, defaultVersion, {}), Error);
|
||||||
WorkerProto::BasicClientConnection::handshake(
|
|
||||||
nullSink, in, defaultVersion, {}),
|
|
||||||
Error);
|
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
});
|
});
|
||||||
|
|
|
@ -2,6 +2,7 @@
|
||||||
///@file
|
///@file
|
||||||
|
|
||||||
#include <memory>
|
#include <memory>
|
||||||
|
#include <type_traits>
|
||||||
|
|
||||||
#include "types.hh"
|
#include "types.hh"
|
||||||
#include "util.hh"
|
#include "util.hh"
|
||||||
|
@ -202,7 +203,14 @@ struct StringSource : Source
|
||||||
{
|
{
|
||||||
std::string_view s;
|
std::string_view s;
|
||||||
size_t pos;
|
size_t pos;
|
||||||
|
|
||||||
|
// NOTE: Prevent unintentional dangling views when an implicit conversion
|
||||||
|
// from std::string -> std::string_view occurs when the string is passed
|
||||||
|
// by rvalue.
|
||||||
|
StringSource(std::string &&) = delete;
|
||||||
StringSource(std::string_view s) : s(s), pos(0) { }
|
StringSource(std::string_view s) : s(s), pos(0) { }
|
||||||
|
StringSource(const std::string& str): StringSource(std::string_view(str)) {}
|
||||||
|
|
||||||
size_t read(char * data, size_t len) override;
|
size_t read(char * data, size_t len) override;
|
||||||
};
|
};
|
||||||
|
|
||||||
|
|
Loading…
Reference in a new issue