Skip to content
Permalink
Browse files

Peek for \n in LineBasedFrameDecoder.

Summary:
Previously this could underflow if there was not a following \n.

CVE-2019-3563

Reviewed By: siyengar

Differential Revision: D14935715

fbshipit-source-id: 25c3eecf373f89efa1232456aeeb092f13b7fa06
  • Loading branch information...
knekritz authored and facebook-github-bot committed Apr 16, 2019
1 parent 1df008b commit 5b3bceca875e4ea4ed9d14c20b20ce46c92c13c6
Showing with 37 additions and 2 deletions.
  1. +3 −2 wangle/codec/LineBasedFrameDecoder.cpp
  2. +34 −0 wangle/codec/test/CodecTest.cpp
@@ -97,8 +97,9 @@ int64_t LineBasedFrameDecoder::findEndOfLine(IOBufQueue& buf) {
auto b = c.read<char>();
if (b == '\n' && terminatorType_ != TerminatorType::CARRIAGENEWLINE) {
return i;
} else if (terminatorType_ != TerminatorType::NEWLINE &&
b == '\r' && !c.isAtEnd() && c.read<char>() == '\n') {
} else if (
terminatorType_ != TerminatorType::NEWLINE && b == '\r' &&
!c.isAtEnd() && *c.peekBytes().data() == '\n') {
return i;
}
}
@@ -606,3 +606,37 @@ TEST(LineBasedFrameDecoder, CarriageNewLineOnly) {
pipeline->read(q);
EXPECT_EQ(called, 1);
}

TEST(LineBasedFrameDecoder, CarriageOnly) {
auto pipeline = Pipeline<IOBufQueue&, std::unique_ptr<IOBuf>>::create();

(*pipeline)
.addBack(LineBasedFrameDecoder(
10, true, LineBasedFrameDecoder::TerminatorType::CARRIAGENEWLINE))
.addBack(test::FrameTester([&](std::unique_ptr<IOBuf>) { FAIL(); }))
.finalize();

IOBufQueue q(IOBufQueue::cacheChainLength());
q.append(IOBuf::copyBuffer("\raa"));
pipeline->read(q);
}

TEST(LineBasedFrameDecoder, DoubleCarriage) {
auto pipeline = Pipeline<IOBufQueue&, std::unique_ptr<IOBuf>>::create();
int called = 0;

(*pipeline)
.addBack(LineBasedFrameDecoder(
10, true, LineBasedFrameDecoder::TerminatorType::CARRIAGENEWLINE))
.addBack(test::FrameTester([&](std::unique_ptr<IOBuf> buf) {
auto sz = buf->computeChainDataLength();
called++;
EXPECT_EQ(sz, 1);
}))
.finalize();

IOBufQueue q(IOBufQueue::cacheChainLength());
q.append(IOBuf::copyBuffer("\r\r\na\r\n"));
pipeline->read(q);
EXPECT_EQ(called, 2);
}

0 comments on commit 5b3bcec

Please sign in to comment.
You can’t perform that action at this time.