Skip to content

Commit c9ce5d2

Browse files
authored
fuzz: track read_disable_count bidirectionally in codec_impl_fuzz_test. (envoyproxy#4260)
Fixes oss-fuzz issue https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=9997. Risk level: Low Testing: Corpus entry added. Signed-off-by: Harvey Tuch <[email protected]>
1 parent 35103b3 commit c9ce5d2

File tree

2 files changed

+38
-37
lines changed

2 files changed

+38
-37
lines changed
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
actions { new_stream { request_headers { headers { key: ":method" value: "�" } headers { key: ":path" value: "�" } headers { key: ":scheme" value: "T" } headers { key: ":authority" value: "T" } } } } actions { client_drain { } } actions { stream_action { request { read_disable: true } } } actions { stream_action { response { read_disable: false } } }

test/common/http/http2/codec_impl_fuzz_test.cc

Lines changed: 37 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -83,72 +83,79 @@ class Stream : public LinkedObject<Stream> {
8383
// buffer level.
8484
enum class StreamState { PendingHeaders, PendingDataOrTrailers, Closed };
8585

86+
struct DirectionalState {
87+
StreamEncoder* encoder_;
88+
NiceMock<MockStreamDecoder> decoder_;
89+
StreamState stream_state_;
90+
uint32_t read_disable_count_{};
91+
} request_, response_;
92+
8693
Stream(TestClientConnectionImpl& client, const TestHeaderMapImpl& request_headers,
87-
bool end_stream)
88-
: request_encoder_(&client.newStream(response_decoder_)) {
94+
bool end_stream) {
95+
request_.encoder_ = &client.newStream(response_.decoder_);
8996
ON_CALL(server_stream_callbacks_, onResetStream(_)).WillByDefault(InvokeWithoutArgs([this] {
90-
request_state_ = response_state_ = StreamState::Closed;
97+
request_.stream_state_ = response_.stream_state_ = StreamState::Closed;
9198
}));
92-
request_encoder_->encodeHeaders(request_headers, end_stream);
93-
request_state_ = end_stream ? StreamState::Closed : StreamState::PendingDataOrTrailers;
94-
response_state_ = StreamState::PendingHeaders;
99+
request_.encoder_->encodeHeaders(request_headers, end_stream);
100+
request_.stream_state_ = end_stream ? StreamState::Closed : StreamState::PendingDataOrTrailers;
101+
response_.stream_state_ = StreamState::PendingHeaders;
95102
}
96103

97104
// Some stream action applied in either the request or resposne direction.
98-
void directionalAction(StreamState& state, StreamEncoder& encoder,
105+
void directionalAction(DirectionalState& state,
99106
const test::common::http::http2::DirectionalAction& directional_action) {
100107
const bool end_stream = directional_action.end_stream();
101108
switch (directional_action.directional_action_selector_case()) {
102109
case test::common::http::http2::DirectionalAction::kContinueHeaders: {
103-
if (state == StreamState::PendingHeaders) {
110+
if (state.stream_state_ == StreamState::PendingHeaders) {
104111
Http::TestHeaderMapImpl headers = Fuzz::fromHeaders(directional_action.continue_headers());
105112
headers.setReferenceKey(Headers::get().Status, "100");
106-
encoder.encode100ContinueHeaders(headers);
113+
state.encoder_->encode100ContinueHeaders(headers);
107114
}
108115
break;
109116
}
110117
case test::common::http::http2::DirectionalAction::kHeaders: {
111-
if (state == StreamState::PendingHeaders) {
112-
encoder.encodeHeaders(Fuzz::fromHeaders(directional_action.headers()), end_stream);
113-
state = end_stream ? StreamState::Closed : StreamState::PendingDataOrTrailers;
118+
if (state.stream_state_ == StreamState::PendingHeaders) {
119+
state.encoder_->encodeHeaders(Fuzz::fromHeaders(directional_action.headers()), end_stream);
120+
state.stream_state_ = end_stream ? StreamState::Closed : StreamState::PendingDataOrTrailers;
114121
}
115122
break;
116123
}
117124
case test::common::http::http2::DirectionalAction::kData: {
118-
if (state == StreamState::PendingDataOrTrailers) {
125+
if (state.stream_state_ == StreamState::PendingDataOrTrailers) {
119126
Buffer::OwnedImpl buf(std::string(directional_action.data() % (1024 * 1024), 'a'));
120-
encoder.encodeData(buf, end_stream);
121-
state = end_stream ? StreamState::Closed : StreamState::PendingDataOrTrailers;
127+
state.encoder_->encodeData(buf, end_stream);
128+
state.stream_state_ = end_stream ? StreamState::Closed : StreamState::PendingDataOrTrailers;
122129
}
123130
break;
124131
}
125132
case test::common::http::http2::DirectionalAction::kTrailers: {
126-
if (state == StreamState::PendingDataOrTrailers) {
127-
encoder.encodeTrailers(Fuzz::fromHeaders(directional_action.trailers()));
128-
state = StreamState::Closed;
133+
if (state.stream_state_ == StreamState::PendingDataOrTrailers) {
134+
state.encoder_->encodeTrailers(Fuzz::fromHeaders(directional_action.trailers()));
135+
state.stream_state_ = StreamState::Closed;
129136
}
130137
break;
131138
}
132139
case test::common::http::http2::DirectionalAction::kResetStream: {
133-
if (state != StreamState::Closed) {
134-
encoder.getStream().resetStream(
140+
if (state.stream_state_ != StreamState::Closed) {
141+
state.encoder_->getStream().resetStream(
135142
static_cast<Http::StreamResetReason>(directional_action.reset_stream()));
136-
request_state_ = response_state_ = StreamState::Closed;
143+
request_.stream_state_ = response_.stream_state_ = StreamState::Closed;
137144
}
138145
break;
139146
}
140147
case test::common::http::http2::DirectionalAction::kReadDisable: {
141-
if (state != StreamState::Closed) {
148+
if (state.stream_state_ != StreamState::Closed) {
142149
const bool disable = directional_action.read_disable();
143-
if (read_disable_count_ == 0 && !disable) {
150+
if (state.read_disable_count_ == 0 && !disable) {
144151
return;
145152
}
146153
if (disable) {
147-
++read_disable_count_;
154+
++state.read_disable_count_;
148155
} else {
149-
--read_disable_count_;
156+
--state.read_disable_count_;
150157
}
151-
encoder.getStream().readDisable(disable);
158+
state.encoder_->getStream().readDisable(disable);
152159
}
153160
break;
154161
}
@@ -161,11 +168,11 @@ class Stream : public LinkedObject<Stream> {
161168
void streamAction(const test::common::http::http2::StreamAction& stream_action) {
162169
switch (stream_action.stream_action_selector_case()) {
163170
case test::common::http::http2::StreamAction::kRequest: {
164-
directionalAction(request_state_, *request_encoder_, stream_action.request());
171+
directionalAction(request_, stream_action.request());
165172
break;
166173
}
167174
case test::common::http::http2::StreamAction::kResponse: {
168-
directionalAction(response_state_, *response_encoder_, stream_action.response());
175+
directionalAction(response_, stream_action.response());
169176
break;
170177
}
171178
default:
@@ -175,13 +182,6 @@ class Stream : public LinkedObject<Stream> {
175182
}
176183

177184
NiceMock<MockStreamCallbacks> server_stream_callbacks_;
178-
NiceMock<MockStreamDecoder> response_decoder_;
179-
StreamEncoder* request_encoder_;
180-
StreamEncoder* response_encoder_;
181-
NiceMock<MockStreamDecoder> request_decoder_;
182-
StreamState request_state_;
183-
StreamState response_state_;
184-
uint32_t read_disable_count_{};
185185
};
186186

187187
// Buffer between client and server H2 codecs. This models each write operation
@@ -283,9 +283,9 @@ DEFINE_PROTO_FUZZER(const test::common::http::http2::CodecImplFuzzTestCase& inpu
283283
auto stream_ptr = pending_streams.front()->removeFromList(pending_streams);
284284
Stream* const stream = stream_ptr.get();
285285
stream_ptr->moveIntoListBack(std::move(stream_ptr), streams);
286-
stream->response_encoder_ = &encoder;
286+
stream->response_.encoder_ = &encoder;
287287
encoder.getStream().addCallbacks(stream->server_stream_callbacks_);
288-
return stream->request_decoder_;
288+
return stream->request_.decoder_;
289289
}));
290290

291291
try {

0 commit comments

Comments
 (0)