Skip to content
Permalink
Browse files Browse the repository at this point in the history
Avoid arithmetic operation on uint16 read from the wire.
Summary:
This could overflow previously.

CVE-2019-3560

Reviewed By: yfeldblum

Differential Revision: D14152362

fbshipit-source-id: c0ebb3fc59b49c7c23e6bcb90458c19cd891be65
  • Loading branch information
knekritz authored and facebook-github-bot committed Feb 26, 2019
1 parent 261b8f6 commit 40bbb16
Show file tree
Hide file tree
Showing 2 changed files with 11 additions and 3 deletions.
4 changes: 1 addition & 3 deletions fizz/record/PlaintextRecordLayer.cpp
Expand Up @@ -39,9 +39,7 @@ folly::Optional<TLSMessage> PlaintextReadRecordLayer::read(
if (buf.chainLength() < (cursor - buf.front()) + length) {
return folly::none;
}
length +=
sizeof(ContentType) + sizeof(ProtocolVersion) + sizeof(uint16_t);
buf.trimStart(length);
buf.trimStart(static_cast<size_t>(kPlaintextHeaderSize) + length);
continue;
} else if (msg.type != ContentType::change_cipher_spec) {
skipEncryptedRecords_ = false;
Expand Down
10 changes: 10 additions & 0 deletions fizz/record/test/PlaintextRecordTest.cpp
Expand Up @@ -115,6 +115,16 @@ TEST_F(PlaintextRecordTest, TestSkipAndWait) {
EXPECT_TRUE(queue_.empty());
}

TEST_F(PlaintextRecordTest, TestSkipOversizedRecord) {
read_.setSkipEncryptedRecords(true);
addToQueue("170301fffb");
auto longBuf = IOBuf::create(0xfffb);
longBuf->append(0xfffb);
queue_.append(std::move(longBuf));
EXPECT_FALSE(read_.read(queue_).hasValue());
EXPECT_TRUE(queue_.empty());
}

TEST_F(PlaintextRecordTest, TestWaitBeforeSkip) {
read_.setSkipEncryptedRecords(true);
addToQueue("170301000501234567");
Expand Down

0 comments on commit 40bbb16

Please sign in to comment.