This is an automated email from the ASF dual-hosted git repository.
maskit pushed a commit to branch quic-05
in repository https://gitbox.apache.org/repos/asf/trafficserver.git
commit 92d7c3a29e988a4a80433b3d27efd3fea6ea1427
Author: Masaori Koshiba <masa...@apache.org>
AuthorDate: Thu Oct 12 16:34:14 2017 +0900
Fix heap-use-after-free in QUICStreamFrame::store
Make data of QUICStramFrame ats_unique_buf. Copy data of _write_vio
to the buffer, when QUICStream sends frame. Ideally this malloc and
copy should be avoided.
(cherry picked from commit 2250a414d8818b5f6b781a9ee9811521d6022e20)
---
iocore/net/quic/QUICFrame.cc | 11 +-
iocore/net/quic/QUICFrame.h | 4 +-
iocore/net/quic/test/test_QUICFrame.cc | 50 +++++-
iocore/net/quic/test/test_QUICFrameDispatcher.cc | 7 +-
iocore/net/quic/test/test_QUICStream.cc | 215 +++++++++++++----------
5 files changed, 174 insertions(+), 113 deletions(-)
diff --git a/iocore/net/quic/QUICFrame.cc b/iocore/net/quic/QUICFrame.cc
index 69951d4..4bb02b4 100644
--- a/iocore/net/quic/QUICFrame.cc
+++ b/iocore/net/quic/QUICFrame.cc
@@ -72,9 +72,9 @@ QUICFrame::reset(const uint8_t *buf, size_t len)
// STREAM Frame
//
-QUICStreamFrame::QUICStreamFrame(const uint8_t *data, size_t data_len,
QUICStreamId stream_id, QUICOffset offset, bool last)
+QUICStreamFrame::QUICStreamFrame(ats_unique_buf data, size_t data_len,
QUICStreamId stream_id, QUICOffset offset, bool last)
{
- this->_data = data;
+ this->_data = std::move(data);
this->_data_len = data_len;
this->_stream_id = stream_id;
this->_offset = offset;
@@ -183,7 +183,7 @@ QUICStreamFrame::data() const
if (this->_buf) {
return this->_buf + this->_get_data_offset();
} else {
- return this->_data;
+ return this->_data.get();
}
}
@@ -1315,8 +1315,11 @@ QUICFrameFactory::fast_create(const uint8_t *buf, size_t
len)
QUICStreamFrameUPtr
QUICFrameFactory::create_stream_frame(const uint8_t *data, size_t data_len,
QUICStreamId stream_id, QUICOffset offset, bool last)
{
+ ats_unique_buf buf = ats_unique_malloc(data_len);
+ memcpy(buf.get(), data, data_len);
+
QUICStreamFrame *frame = quicStreamFrameAllocator.alloc();
- new (frame) QUICStreamFrame(data, data_len, stream_id, offset, last);
+ new (frame) QUICStreamFrame(std::move(buf), data_len, stream_id, offset,
last);
return QUICStreamFrameUPtr(frame, &QUICFrameDeleter::delete_stream_frame);
}
diff --git a/iocore/net/quic/QUICFrame.h b/iocore/net/quic/QUICFrame.h
index 772d65a..9f2b356 100644
--- a/iocore/net/quic/QUICFrame.h
+++ b/iocore/net/quic/QUICFrame.h
@@ -59,7 +59,7 @@ class QUICStreamFrame : public QUICFrame
public:
QUICStreamFrame() : QUICFrame() {}
QUICStreamFrame(const uint8_t *buf, size_t len) : QUICFrame(buf, len) {}
- QUICStreamFrame(const uint8_t *buf, size_t len, QUICStreamId streamid,
QUICOffset offset, bool last = false);
+ QUICStreamFrame(ats_unique_buf buf, size_t len, QUICStreamId streamid,
QUICOffset offset, bool last = false);
virtual QUICFrameType type() const override;
virtual size_t size() const override;
virtual void store(uint8_t *buf, size_t *len) const override;
@@ -74,7 +74,7 @@ public:
LINK(QUICStreamFrame, link);
private:
- const uint8_t *_data = nullptr;
+ ats_unique_buf _data = {nullptr, [](void *p) { ats_free(p); }};
size_t _data_len = 0;
QUICStreamId _stream_id = 0;
QUICOffset _offset = 0;
diff --git a/iocore/net/quic/test/test_QUICFrame.cc
b/iocore/net/quic/test/test_QUICFrame.cc
index 90c82fb..36f1eee 100644
--- a/iocore/net/quic/test/test_QUICFrame.cc
+++ b/iocore/net/quic/test/test_QUICFrame.cc
@@ -55,11 +55,14 @@ TEST_CASE("QUICFrame Type", "[quic]")
TEST_CASE("Construct QUICFrame", "[quic]")
{
- uint8_t payload[] = "foo";
+ uint8_t raw[] = "foo";
+ ats_unique_buf payload = ats_unique_malloc(sizeof(raw));
+ memcpy(payload.get(), raw, sizeof(raw));
+
uint8_t buf[65536];
size_t len;
- QUICStreamFrame frame1(payload, sizeof(payload), 0xffcc9966,
0xffddbb9977553311);
+ QUICStreamFrame frame1(std::move(payload), sizeof(raw), 0xffcc9966,
0xffddbb9977553311);
frame1.store(buf, &len);
CHECK(frame1.type() == QUICFrameType::STREAM);
CHECK(frame1.size() == 19);
@@ -119,7 +122,12 @@ TEST_CASE("Store STREAM Frame", "[quic]")
0x00, 0x05, // Data Length
0x01, 0x02, 0x03, 0x04, 0x05, // Stream Data
};
- QUICStreamFrame streamFrame1(reinterpret_cast<const uint8_t
*>("\x01\x02\x03\x04\x05"), 5, 0x01, 0x00);
+
+ uint8_t raw1[] = "\x01\x02\x03\x04\x05";
+ ats_unique_buf payload1 = ats_unique_malloc(5);
+ memcpy(payload1.get(), raw1, 5);
+
+ QUICStreamFrame streamFrame1(std::move(payload1), 5, 0x01, 0x00);
streamFrame1.store(buf, &len);
CHECK(len == 9);
CHECK(memcmp(buf, expected1, len) == 0);
@@ -132,7 +140,11 @@ TEST_CASE("Store STREAM Frame", "[quic]")
0x00, 0x05, // Data Length
0x01, 0x02, 0x03, 0x04, 0x05, // Stream Data
};
- QUICStreamFrame streamFrame2(reinterpret_cast<const uint8_t
*>("\x01\x02\x03\x04\x05"), 5, 0x01, 0x01);
+ uint8_t raw2[] = "\x01\x02\x03\x04\x05";
+ ats_unique_buf payload2 = ats_unique_malloc(5);
+ memcpy(payload2.get(), raw2, 5);
+
+ QUICStreamFrame streamFrame2(std::move(payload2), 5, 0x01, 0x01);
streamFrame2.store(buf, &len);
CHECK(len == 11);
CHECK(memcmp(buf, expected2, len) == 0);
@@ -145,7 +157,11 @@ TEST_CASE("Store STREAM Frame", "[quic]")
0x00, 0x05, // Data Length
0x01, 0x02, 0x03, 0x04, 0x05, // Stream Data
};
- QUICStreamFrame streamFrame3(reinterpret_cast<const uint8_t
*>("\x01\x02\x03\x04\x05"), 5, 0x01, 0x010000);
+ uint8_t raw3[] = "\x01\x02\x03\x04\x05";
+ ats_unique_buf payload3 = ats_unique_malloc(5);
+ memcpy(payload3.get(), raw3, 5);
+
+ QUICStreamFrame streamFrame3(std::move(payload3), 5, 0x01, 0x010000);
streamFrame3.store(buf, &len);
CHECK(len == 13);
CHECK(memcmp(buf, expected3, len) == 0);
@@ -158,7 +174,11 @@ TEST_CASE("Store STREAM Frame", "[quic]")
0x00, 0x05, // Data Length
0x01, 0x02, 0x03, 0x04, 0x05, // Stream Data
};
- QUICStreamFrame streamFrame4(reinterpret_cast<const uint8_t
*>("\x01\x02\x03\x04\x05"), 5, 0x01, 0x0100000000);
+ uint8_t raw4[] = "\x01\x02\x03\x04\x05";
+ ats_unique_buf payload4 = ats_unique_malloc(5);
+ memcpy(payload4.get(), raw4, 5);
+
+ QUICStreamFrame streamFrame4(std::move(payload4), 5, 0x01, 0x0100000000);
streamFrame4.store(buf, &len);
CHECK(len == 17);
CHECK(memcmp(buf, expected4, len) == 0);
@@ -171,7 +191,11 @@ TEST_CASE("Store STREAM Frame", "[quic]")
0x00, 0x05, // Data Length
0x01, 0x02, 0x03, 0x04, 0x05, // Stream Data
};
- QUICStreamFrame streamFrame5(reinterpret_cast<const uint8_t
*>("\x01\x02\x03\x04\x05"), 5, 0x0100, 0x0100000000);
+ uint8_t raw5[] = "\x01\x02\x03\x04\x05";
+ ats_unique_buf payload5 = ats_unique_malloc(5);
+ memcpy(payload5.get(), raw5, 5);
+
+ QUICStreamFrame streamFrame5(std::move(payload5), 5, 0x0100, 0x0100000000);
streamFrame5.store(buf, &len);
CHECK(len == 18);
CHECK(memcmp(buf, expected5, len) == 0);
@@ -184,7 +208,11 @@ TEST_CASE("Store STREAM Frame", "[quic]")
0x00, 0x05, // Data Length
0x01, 0x02, 0x03, 0x04, 0x05, // Stream Data
};
- QUICStreamFrame streamFrame6(reinterpret_cast<const uint8_t
*>("\x01\x02\x03\x04\x05"), 5, 0x010000, 0x0100000000);
+ uint8_t raw6[] = "\x01\x02\x03\x04\x05";
+ ats_unique_buf payload6 = ats_unique_malloc(5);
+ memcpy(payload6.get(), raw6, 5);
+
+ QUICStreamFrame streamFrame6(std::move(payload6), 5, 0x010000, 0x0100000000);
streamFrame6.store(buf, &len);
CHECK(len == 19);
CHECK(memcmp(buf, expected6, len) == 0);
@@ -197,7 +225,11 @@ TEST_CASE("Store STREAM Frame", "[quic]")
0x00, 0x05, // Data Length
0x01, 0x02, 0x03, 0x04, 0x05, // Stream Data
};
- QUICStreamFrame streamFrame7(reinterpret_cast<const uint8_t
*>("\x01\x02\x03\x04\x05"), 5, 0x01000000, 0x0100000000);
+ uint8_t raw7[] = "\x01\x02\x03\x04\x05";
+ ats_unique_buf payload7 = ats_unique_malloc(5);
+ memcpy(payload7.get(), raw7, 5);
+
+ QUICStreamFrame streamFrame7(std::move(payload7), 5, 0x01000000,
0x0100000000);
streamFrame7.store(buf, &len);
CHECK(len == 20);
CHECK(memcmp(buf, expected7, len) == 0);
diff --git a/iocore/net/quic/test/test_QUICFrameDispatcher.cc
b/iocore/net/quic/test/test_QUICFrameDispatcher.cc
index 8573dce..215303f 100644
--- a/iocore/net/quic/test/test_QUICFrameDispatcher.cc
+++ b/iocore/net/quic/test/test_QUICFrameDispatcher.cc
@@ -29,8 +29,11 @@
TEST_CASE("QUICFrameHandler", "[quic]")
{
- uint8_t payload[] = {0x01};
- QUICStreamFrame streamFrame(payload, 1, 0x03, 0);
+ uint8_t raw[] = {0x01};
+ ats_unique_buf payload = ats_unique_malloc(1);
+ memcpy(payload.get(), raw, 1);
+
+ QUICStreamFrame streamFrame(std::move(payload), 1, 0x03, 0);
auto connection = new MockQUICConnection();
auto streamManager = new MockQUICStreamManager();
diff --git a/iocore/net/quic/test/test_QUICStream.cc
b/iocore/net/quic/test/test_QUICStream.cc
index 632ad91..ef523ab 100644
--- a/iocore/net/quic/test/test_QUICStream.cc
+++ b/iocore/net/quic/test/test_QUICStream.cc
@@ -26,101 +26,124 @@
#include "quic/QUICStream.h"
#include "quic/Mock.h"
-namespace
+TEST_CASE("QUICStream", "[quic]")
{
-// Test Data
-uint8_t payload[] = {0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07, 0x08, 0x09,
0x0a, 0x0b, 0x0c, 0x0d, 0x0e, 0x0f, 0x10};
-uint32_t stream_id = 0x03;
-
-std::shared_ptr<QUICStreamFrame> frame_1 =
std::make_shared<QUICStreamFrame>(payload, 2, stream_id, 0);
-std::shared_ptr<QUICStreamFrame> frame_2 =
std::make_shared<QUICStreamFrame>(payload + 2, 2, stream_id, 2);
-std::shared_ptr<QUICStreamFrame> frame_3 =
std::make_shared<QUICStreamFrame>(payload + 4, 2, stream_id, 4);
-std::shared_ptr<QUICStreamFrame> frame_4 =
std::make_shared<QUICStreamFrame>(payload + 6, 2, stream_id, 6);
-std::shared_ptr<QUICStreamFrame> frame_5 =
std::make_shared<QUICStreamFrame>(payload + 8, 2, stream_id, 8);
-std::shared_ptr<QUICStreamFrame> frame_6 =
std::make_shared<QUICStreamFrame>(payload + 10, 2, stream_id, 10);
-std::shared_ptr<QUICStreamFrame> frame_7 =
std::make_shared<QUICStreamFrame>(payload + 12, 2, stream_id, 12);
-std::shared_ptr<QUICStreamFrame> frame_8 =
std::make_shared<QUICStreamFrame>(payload + 14, 2, stream_id, 14);
-
-TEST_CASE("QUICStream_assembling_byte_stream_1", "[quic]")
-{
- MIOBuffer *read_buffer = new_MIOBuffer(BUFFER_SIZE_INDEX_4K);
- IOBufferReader *reader = read_buffer->alloc_reader();
- MockQUICFrameTransmitter tx;
-
- std::unique_ptr<QUICStream> stream(new QUICStream());
- stream->init(&tx, 0, stream_id, 1024, 1024);
- stream->do_io_read(nullptr, 0, read_buffer);
-
- stream->recv(frame_1);
- stream->recv(frame_2);
- stream->recv(frame_3);
- stream->recv(frame_4);
- stream->recv(frame_5);
- stream->recv(frame_6);
- stream->recv(frame_7);
- stream->recv(frame_8);
-
- uint8_t buf[32];
- int64_t len = reader->read_avail();
- reader->read(buf, len);
-
- CHECK(len == 16);
- CHECK(memcmp(buf, payload, len) == 0);
-}
-
-TEST_CASE("QUICStream_assembling_byte_stream_2", "[quic]")
-{
- MIOBuffer *read_buffer = new_MIOBuffer(BUFFER_SIZE_INDEX_4K);
- IOBufferReader *reader = read_buffer->alloc_reader();
- MockQUICFrameTransmitter tx;
-
- std::unique_ptr<QUICStream> stream(new QUICStream());
- stream->init(&tx, 0, stream_id);
- stream->do_io_read(nullptr, 0, read_buffer);
-
- stream->recv(frame_8);
- stream->recv(frame_7);
- stream->recv(frame_6);
- stream->recv(frame_5);
- stream->recv(frame_4);
- stream->recv(frame_3);
- stream->recv(frame_2);
- stream->recv(frame_1);
-
- uint8_t buf[32];
- int64_t len = reader->read_avail();
- reader->read(buf, len);
-
- CHECK(len == 16);
- CHECK(memcmp(buf, payload, len) == 0);
-}
-
-TEST_CASE("QUICStream_assembling_byte_stream_3", "[quic]")
-{
- MIOBuffer *read_buffer = new_MIOBuffer(BUFFER_SIZE_INDEX_4K);
- IOBufferReader *reader = read_buffer->alloc_reader();
- MockQUICFrameTransmitter tx;
-
- std::unique_ptr<QUICStream> stream(new QUICStream());
- stream->init(&tx, 0, stream_id);
- stream->do_io_read(nullptr, 0, read_buffer);
-
- stream->recv(frame_8);
- stream->recv(frame_7);
- stream->recv(frame_6);
- stream->recv(frame_7); // duplicated frame
- stream->recv(frame_5);
- stream->recv(frame_3);
- stream->recv(frame_1);
- stream->recv(frame_2);
- stream->recv(frame_4);
- stream->recv(frame_5); // duplicated frame
-
- uint8_t buf[32];
- int64_t len = reader->read_avail();
- reader->read(buf, len);
-
- CHECK(len == 16);
- CHECK(memcmp(buf, payload, len) == 0);
-}
+ // Test Data
+ uint8_t payload[] = {0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07, 0x08, 0x09,
0x0a, 0x0b, 0x0c, 0x0d, 0x0e, 0x0f, 0x10};
+ uint32_t stream_id = 0x03;
+
+ ats_unique_buf payload1 = ats_unique_malloc(2);
+ memcpy(payload1.get(), payload, 2);
+ std::shared_ptr<QUICStreamFrame> frame_1 =
std::make_shared<QUICStreamFrame>(std::move(payload1), 2, stream_id, 0);
+
+ ats_unique_buf payload2 = ats_unique_malloc(2);
+ memcpy(payload2.get(), payload + 2, 2);
+ std::shared_ptr<QUICStreamFrame> frame_2 =
std::make_shared<QUICStreamFrame>(std::move(payload2), 2, stream_id, 2);
+
+ ats_unique_buf payload3 = ats_unique_malloc(2);
+ memcpy(payload3.get(), payload + 4, 2);
+ std::shared_ptr<QUICStreamFrame> frame_3 =
std::make_shared<QUICStreamFrame>(std::move(payload3), 2, stream_id, 4);
+
+ ats_unique_buf payload4 = ats_unique_malloc(2);
+ memcpy(payload4.get(), payload + 6, 2);
+ std::shared_ptr<QUICStreamFrame> frame_4 =
std::make_shared<QUICStreamFrame>(std::move(payload4), 2, stream_id, 6);
+
+ ats_unique_buf payload5 = ats_unique_malloc(2);
+ memcpy(payload5.get(), payload + 8, 2);
+ std::shared_ptr<QUICStreamFrame> frame_5 =
std::make_shared<QUICStreamFrame>(std::move(payload5), 2, stream_id, 8);
+
+ ats_unique_buf payload6 = ats_unique_malloc(2);
+ memcpy(payload6.get(), payload + 10, 2);
+ std::shared_ptr<QUICStreamFrame> frame_6 =
std::make_shared<QUICStreamFrame>(std::move(payload6), 2, stream_id, 10);
+
+ ats_unique_buf payload7 = ats_unique_malloc(2);
+ memcpy(payload7.get(), payload + 12, 2);
+ std::shared_ptr<QUICStreamFrame> frame_7 =
std::make_shared<QUICStreamFrame>(std::move(payload7), 2, stream_id, 12);
+
+ ats_unique_buf payload8 = ats_unique_malloc(2);
+ memcpy(payload8.get(), payload + 14, 2);
+ std::shared_ptr<QUICStreamFrame> frame_8 =
std::make_shared<QUICStreamFrame>(std::move(payload8), 2, stream_id, 14);
+
+ SECTION("QUICStream_assembling_byte_stream_1")
+ {
+ MIOBuffer *read_buffer = new_MIOBuffer(BUFFER_SIZE_INDEX_4K);
+ IOBufferReader *reader = read_buffer->alloc_reader();
+ MockQUICFrameTransmitter tx;
+
+ std::unique_ptr<QUICStream> stream(new QUICStream());
+ stream->init(&tx, 0, stream_id, 1024, 1024);
+ stream->do_io_read(nullptr, 0, read_buffer);
+
+ stream->recv(frame_1);
+ stream->recv(frame_2);
+ stream->recv(frame_3);
+ stream->recv(frame_4);
+ stream->recv(frame_5);
+ stream->recv(frame_6);
+ stream->recv(frame_7);
+ stream->recv(frame_8);
+
+ uint8_t buf[32];
+ int64_t len = reader->read_avail();
+ reader->read(buf, len);
+
+ CHECK(len == 16);
+ CHECK(memcmp(buf, payload, len) == 0);
+ }
+
+ SECTION("QUICStream_assembling_byte_stream_2")
+ {
+ MIOBuffer *read_buffer = new_MIOBuffer(BUFFER_SIZE_INDEX_4K);
+ IOBufferReader *reader = read_buffer->alloc_reader();
+ MockQUICFrameTransmitter tx;
+
+ std::unique_ptr<QUICStream> stream(new QUICStream());
+ stream->init(&tx, 0, stream_id);
+ stream->do_io_read(nullptr, 0, read_buffer);
+
+ stream->recv(frame_8);
+ stream->recv(frame_7);
+ stream->recv(frame_6);
+ stream->recv(frame_5);
+ stream->recv(frame_4);
+ stream->recv(frame_3);
+ stream->recv(frame_2);
+ stream->recv(frame_1);
+
+ uint8_t buf[32];
+ int64_t len = reader->read_avail();
+ reader->read(buf, len);
+
+ CHECK(len == 16);
+ CHECK(memcmp(buf, payload, len) == 0);
+ }
+
+ SECTION("QUICStream_assembling_byte_stream_3")
+ {
+ MIOBuffer *read_buffer = new_MIOBuffer(BUFFER_SIZE_INDEX_4K);
+ IOBufferReader *reader = read_buffer->alloc_reader();
+ MockQUICFrameTransmitter tx;
+
+ std::unique_ptr<QUICStream> stream(new QUICStream());
+ stream->init(&tx, 0, stream_id);
+ stream->do_io_read(nullptr, 0, read_buffer);
+
+ stream->recv(frame_8);
+ stream->recv(frame_7);
+ stream->recv(frame_6);
+ stream->recv(frame_7); // duplicated frame
+ stream->recv(frame_5);
+ stream->recv(frame_3);
+ stream->recv(frame_1);
+ stream->recv(frame_2);
+ stream->recv(frame_4);
+ stream->recv(frame_5); // duplicated frame
+
+ uint8_t buf[32];
+ int64_t len = reader->read_avail();
+ reader->read(buf, len);
+
+ CHECK(len == 16);
+ CHECK(memcmp(buf, payload, len) == 0);
+ }
}
--
To stop receiving notification emails like this one, please contact
"commits@trafficserver.apache.org" <commits@trafficserver.apache.org>.
