Skip to content

Commit

Permalink
Bug#35812604 heap-use-after-free after read() fail
Browse files Browse the repository at this point in the history
ASAN reports a use-after-free when handling queries larger than 16
Mbyte.

Background
==========

The use-after-free happens when using the read_buffer_view() after

- Channel::read_to_plain() grew the read-buffer
- the socket-read() failed
- the read-buffer was resized to its original size.

Change
======

- sync the read_buffer_view() even if the read() failed as it may have
  moved the read-buffer to a new memory area.

Change-Id: I2511052394e83d217e4349557ec00664472b48da
  • Loading branch information
weigon authored and dahlerlend committed Sep 20, 2023
1 parent d71a96a commit dd2f070
Show file tree
Hide file tree
Showing 2 changed files with 10 additions and 7 deletions.
5 changes: 4 additions & 1 deletion router/src/routing/src/channel.cc
Original file line number Diff line number Diff line change
Expand Up @@ -238,13 +238,16 @@ stdx::expected<size_t, std::error_code> Channel::read_to_plain(size_t sz) {

// append to the plain buffer
const auto read_res = read(dyn_buf, sz);

// sync the plain-view as the read() may have resized it.
view_sync_plain();

if (read_res) {
const size_t transferred = read_res.value();

sz -= transferred;
bytes_read += transferred;

view_sync_plain();
} else {
// read from client failed.

Expand Down
12 changes: 6 additions & 6 deletions router/src/routing/src/classic_frame.cc
Original file line number Diff line number Diff line change
Expand Up @@ -170,8 +170,6 @@ ClassicFrame::ensure_has_full_frame(Channel *src_channel,
[[nodiscard]] stdx::expected<size_t, std::error_code>
ClassicFrame::recv_frame_sequence(Channel *src_channel,
ClassicProtocolState *src_protocol) {
auto &recv_buf = src_channel->recv_plain_view();

bool expect_header{true}; // toggle between header and payload
const size_t hdr_size{4};
size_t expected_size{hdr_size};
Expand All @@ -182,17 +180,19 @@ ClassicFrame::recv_frame_sequence(Channel *src_channel,
src_protocol->current_frame().reset();

for (;;) {
auto recv_buf_size = src_channel->recv_plain_view().size();

// fill the recv-buf with the expected bytes.
if (recv_buf.size() < expected_size) {
auto read_res =
src_channel->read_to_plain(expected_size - recv_buf.size());
if (recv_buf_size < expected_size) {
auto read_res = src_channel->read_to_plain(expected_size - recv_buf_size);
if (!read_res) return read_res.get_unexpected();

if (recv_buf.size() < expected_size) {
if (src_channel->recv_plain_view().size() < expected_size) {
return stdx::make_unexpected(make_error_code(TlsErrc::kWantRead));
}
}

auto &recv_buf = src_channel->recv_plain_view();
if (expect_header) {
const auto hdr_res =
classic_protocol::decode<classic_protocol::frame::Header>(
Expand Down

0 comments on commit dd2f070

Please sign in to comment.