Permalink
Browse files

[security][CVE-2018-6335] Fix potential crash in HTTP2 padding handling

CVE-2018-6335
  • Loading branch information...
fredemmott committed May 3, 2018
1 parent 2e4b6ed commit 4cb57dd753a339654ca464c139db9871fe961d56
Showing with 100 additions and 0 deletions.
  1. +100 −0 patches/proxygen-CVE-2018-6335.patch
@@ -0,0 +1,100 @@
diff --git a/third-party/proxygen/src/proxygen/lib/http/codec/HTTP2Framer.cpp b/third-party/proxygen/src/proxygen/lib/http/codec/HTTP2Framer.cpp
index 068fea4b..9dc0c994 100644
--- a/third-party/proxygen/src/proxygen/lib/http/codec/HTTP2Framer.cpp
+++ b/third-party/proxygen/src/proxygen/lib/http/codec/HTTP2Framer.cpp
@@ -187,6 +187,11 @@ parsePadding(Cursor& cursor,
} else {
padding = 0;
}
+
+ if (header.length < padding) {
+ return ErrorCode::PROTOCOL_ERROR;
+ }
+ header.length -= padding;
return ErrorCode::NO_ERROR;
}
@@ -259,7 +264,7 @@ parseData(Cursor& cursor,
// outPadding is the total number of flow-controlled pad bytes, which
// includes the length byte, if present.
outPadding = padding + ((frameHasPadding(header)) ? 1 : 0);
- cursor.clone(outBuf, header.length - padding);
+ cursor.clone(outBuf, header.length);
return skipPadding(cursor, padding, kStrictPadding);
}
@@ -313,7 +318,7 @@ parseHeaders(Cursor& cursor,
if (header.length < padding) {
return ErrorCode::PROTOCOL_ERROR;
}
- cursor.clone(outBuf, header.length - padding);
+ cursor.clone(outBuf, header.length);
return skipPadding(cursor, padding, kStrictPadding);
}
@@ -398,7 +403,7 @@ parsePushPromise(Cursor& cursor,
if (header.length < padding) {
return ErrorCode::PROTOCOL_ERROR;
}
- cursor.clone(outBuf, header.length - padding);
+ cursor.clone(outBuf, header.length);
return skipPadding(cursor, padding, kStrictPadding);
}
@@ -470,7 +475,7 @@ parseContinuation(Cursor& cursor,
if (header.length < padding) {
return ErrorCode::PROTOCOL_ERROR;
}
- cursor.clone(outBuf, header.length - padding);
+ cursor.clone(outBuf, header.length);
return skipPadding(cursor, padding, kStrictPadding);
}
diff --git a/third-party/proxygen/src/proxygen/lib/http/codec/test/HTTP2CodecTest.cpp b/third-party/proxygen/src/proxygen/lib/http/codec/test/HTTP2CodecTest.cpp
index 97008421..a859861c 100644
--- a/third-party/proxygen/src/proxygen/lib/http/codec/test/HTTP2CodecTest.cpp
+++ b/third-party/proxygen/src/proxygen/lib/http/codec/test/HTTP2CodecTest.cpp
@@ -638,15 +638,17 @@ TEST_F(HTTP2CodecTest, MalformedPaddingLength) {
output_.append(badInput, sizeof(badInput));
EXPECT_EQ(output_.chainLength(), sizeof(badInput));
- bool caughtException = false;
- bool parseResult = true;
- try {
- parseResult = parse();
- } catch (const std::exception &e) {
- caughtException = true;
- }
- EXPECT_FALSE(caughtException);
- EXPECT_FALSE(parseResult);
+ EXPECT_FALSE(parse());
+}
+
+TEST_F(HTTP2CodecTest, MalformedPadding) {
+ const uint8_t badInput[] = {
+ 0x00, 0x00, 0x0d, 0x01, 0xbe, 0x63, 0x0d, 0x0a, 0x0d, 0x0a, 0x00, 0x73,
+ 0x00, 0x00, 0x06, 0x08, 0x72, 0x00, 0x24, 0x00, 0xfa, 0x4d, 0x0d
+ };
+ output_.append(badInput, sizeof(badInput));
+
+ EXPECT_FALSE(parse());
}
TEST_F(HTTP2CodecTest, NoAppByte) {
@@ -659,15 +661,7 @@ TEST_F(HTTP2CodecTest, NoAppByte) {
output_.append(noAppByte, sizeof(noAppByte));
EXPECT_EQ(output_.chainLength(), sizeof(noAppByte));
- bool caughtException = false;
- bool parseResult = false;
- try {
- parseResult = parse();
- } catch (const std::exception &e) {
- caughtException = true;
- }
- EXPECT_FALSE(caughtException);
- EXPECT_TRUE(parseResult);
+ EXPECT_TRUE(parse());
EXPECT_EQ(callbacks_.messageBegin, 0);
EXPECT_EQ(callbacks_.headersComplete, 0);
EXPECT_EQ(callbacks_.messageComplete, 0);

0 comments on commit 4cb57dd

Please sign in to comment.