patches/0001-CVE-2019-3552-potential-DoS-in-Thrift.patch

   1 From 68001a9687dae0b658515aaa3e662e39603e7167 Mon Sep 17 00:00:00 2001
   2 From: Fred Emmott <fe@fb.com>
   3 Date: Thu, 7 Feb 2019 14:48:23 -0800
   4 Subject: [PATCH] CVE-2019-3552 - potential DoS in Thrift
   5
   6 ---
   7  patches/CVE-2019-3552-hhvm.patch   |  31 +++++++
   8  patches/CVE-2019-3552-thrift.patch | 170 +++++++++++++++++++++++++++++++++++++
   9  2 files changed, 201 insertions(+)
  10  create mode 100644 patches/CVE-2019-3552-hhvm.patch
  11  create mode 100644 patches/CVE-2019-3552-thrift.patch
  12
  13 diff --git a/patches/CVE-2019-3552-hhvm.patch b/patches/CVE-2019-3552-hhvm.patch
  14 new file mode 100644
  15 index 0000000000..585c7b89f2
  16 --- /dev/null
  17 +++ b/patches/CVE-2019-3552-hhvm.patch
  18 @@ -0,0 +1,31 @@
  19 +From PHID-DIFF-sj5hbmb7vylhpaqgrb7c Mon Sep 17 00:00:00 2001
  20 +From: Stepan Palamarchuk <stepan@fb.com>
  21 +Date: Tue, 22 Jan 2019 09:54:52 -0800
  22 +Subject: [PATCH] Throw on bad types during skipping data
  23 +
  24 +Summary:
  25 +The current code silently returns on bad types. In case when we have an invalid data, we may get a container of a large size with a bad type, this would lead to us running long loop doing nothing (though we already can say that the data is invalid).
  26 +
  27 +The new code would throw an exception as soon as we try to skip a value of invalid type.
  28 +
  29 +Fixes CVE-2019-3552
  30 +
  31 +Reviewed By: yfeldblum, stevegury
  32 +
  33 +Differential Revision: D8344920
  34 +---
  35 +
  36 +diff --git a/hphp/runtime/ext/thrift/compact.cpp b/hphp/runtime/ext/thrift/compact.cpp
  37 +--- a/hphp/runtime/ext/thrift/compact.cpp
  38 ++++ b/hphp/runtime/ext/thrift/compact.cpp
  39 +@@ -865,6 +865,8 @@
  40 +       switch (type) {
  41 +         case T_STOP:
  42 +         case T_VOID:
  43 ++          thrift_error("Encountered invalid type for skipping T_STOP/T_VOID",
  44 ++                       ERR_INVALID_DATA);
  45 +           break;
  46 +
  47 +         case T_STRUCT: {
  48 +--
  49 +1.7.9.5
  50 diff --git a/patches/CVE-2019-3552-thrift.patch b/patches/CVE-2019-3552-thrift.patch
  51 new file mode 100644
  52 index 0000000000..4a2ac40f08
  53 --- /dev/null
  54 +++ b/patches/CVE-2019-3552-thrift.patch
  55 @@ -0,0 +1,170 @@
  56 +From 5a86b29f56048f635abb443ccc9f1a3dc61b4a9c Mon Sep 17 00:00:00 2001
  57 +From: Stepan Palamarchuk <stepan@fb.com>
  58 +Date: Tue, 22 Jan 2019 09:54:52 -0800
  59 +Subject: [PATCH] Throw on bad types during skipping data
  60 +
  61 +Summary:
  62 +The current code silently returns on bad types. In case when we have an invalid data, we may get a container of a large size with a bad type, this would lead to us running long loop doing nothing (though we already can say that the data is invalid).
  63 +
  64 +The new code would throw an exception as soon as we try to skip a value of invalid type.
  65 +
  66 +Fixes CVE-2019-3552
  67 +
  68 +Reviewed By: yfeldblum, stevegury
  69 +
  70 +Differential Revision: D8344920
  71 +---
  72 + thrift/lib/cpp/protocol/TProtocolException.cpp |  8 +++
  73 + thrift/lib/cpp/protocol/TProtocolException.h   |  2 +
  74 + thrift/lib/cpp2/protocol/Protocol.h            |  5 +-
  75 + thrift/lib/py/protocol/TProtocol.py            |  5 +-
  76 + thrift/test/ProtocolSkipTest.cpp               | 68 ++++++++++++++++++++++++++
  77 + 5 files changed, 85 insertions(+), 3 deletions(-)
  78 + create mode 100644 thrift/test/ProtocolSkipTest.cpp
  79 +
  80 +diff --git a/third-party/thrift/src/thrift/lib/cpp/protocol/TProtocolException.cpp b/third-party/thrift/src/thrift/lib/cpp/protocol/TProtocolException.cpp
  81 +index d0ad89450..ab84de2a7 100644
  82 +--- a/third-party/thrift/src/thrift/lib/cpp/protocol/TProtocolException.cpp
  83 ++++ b/third-party/thrift/src/thrift/lib/cpp/protocol/TProtocolException.cpp
  84 +@@ -57,4 +57,12 @@ namespace apache { namespace thrift { namespace protocol {
  85 +           "Attempt to interpret value {} as bool, probably the data is corrupted",
  86 +           value));
  87 + }
  88 ++
  89 ++[[noreturn]] void TProtocolException::throwInvalidSkipType(TType type) {
  90 ++  throw TProtocolException(
  91 ++      TProtocolException::INVALID_DATA,
  92 ++      folly::sformat(
  93 ++          "Encountered invalid field/element type ({}) during skipping",
  94 ++          static_cast<uint8_t>(type)));
  95 ++}
  96 + }}}
  97 +diff --git a/third-party/thrift/src/thrift/lib/cpp/protocol/TProtocolException.h b/third-party/thrift/src/thrift/lib/cpp/protocol/TProtocolException.h
  98 +index 874654fa4..193d25d78 100644
  99 +--- a/third-party/thrift/src/thrift/lib/cpp/protocol/TProtocolException.h
 100 ++++ b/third-party/thrift/src/thrift/lib/cpp/protocol/TProtocolException.h
 101 +@@ -23,6 +23,7 @@
 102 + #define _THRIFT_PROTOCOL_TPROTOCOLEXCEPTION_H_ 1
 103 +
 104 + #include <thrift/lib/cpp/Thrift.h>
 105 ++#include <thrift/lib/cpp/protocol/TType.h>
 106 +
 107 + #include <string>
 108 +
 109 +@@ -106,6 +107,7 @@ class TProtocolException : public apache::thrift::TLibraryException {
 110 +       folly::StringPiece field,
 111 +       folly::StringPiece type);
 112 +   [[noreturn]] static void throwBoolValueOutOfRange(uint8_t value);
 113 ++  [[noreturn]] static void throwInvalidSkipType(TType type);
 114 +
 115 +  protected:
 116 +   /**
 117 +diff --git a/third-party/thrift/src/thrift/lib/cpp2/protocol/Protocol.h b/third-party/thrift/src/thrift/lib/cpp2/protocol/Protocol.h
 118 +index d1cc63ed6..033668ac7 100644
 119 +--- a/third-party/thrift/src/thrift/lib/cpp2/protocol/Protocol.h
 120 ++++ b/third-party/thrift/src/thrift/lib/cpp2/protocol/Protocol.h
 121 +@@ -176,8 +176,9 @@ void skip(Protocol_& prot, TType arg_type) {
 122 +       prot.readListEnd();
 123 +       return;
 124 +     }
 125 +-    default:
 126 +-      return;
 127 ++    default: {
 128 ++      TProtocolException::throwInvalidSkipType(arg_type);
 129 ++    }
 130 +   }
 131 + }
 132 +
 133 +diff --git a/third-party/thrift/src/thrift/lib/py/protocol/TProtocol.py b/third-party/thrift/src/thrift/lib/py/protocol/TProtocol.py
 134 +index a229eb025..f7f55bbe2 100644
 135 +--- a/third-party/thrift/src/thrift/lib/py/protocol/TProtocol.py
 136 ++++ b/third-party/thrift/src/thrift/lib/py/protocol/TProtocol.py
 137 +@@ -178,7 +178,10 @@ class TProtocolBase:
 138 +
 139 +     def skip(self, type):
 140 +         if type == TType.STOP:
 141 +-            return
 142 ++            raise TProtocolException(
 143 ++                TProtocolException.INVALID_DATA,
 144 ++                "Unexpected type for skipping T_STOP"
 145 ++            )
 146 +         elif type == TType.BOOL:
 147 +             self.readBool()
 148 +         elif type == TType.BYTE:
 149 +diff --git a/third-party/thrift/src/thrift/test/ProtocolSkipTest.cpp b/third-party/thrift/src/thrift/test/ProtocolSkipTest.cpp
 150 +new file mode 100644
 151 +index 000000000..762ec64a8
 152 +--- /dev/null
 153 ++++ b/third-party/thrift/src/thrift/test/ProtocolSkipTest.cpp
 154 +@@ -0,0 +1,68 @@
 155 ++/*
 156 ++ * Copyright 2004-present Facebook, Inc.
 157 ++ *
 158 ++ * Licensed under the Apache License, Version 2.0 (the "License");
 159 ++ * you may not use this file except in compliance with the License.
 160 ++ * You may obtain a copy of the License at
 161 ++ *
 162 ++ *   http://www.apache.org/licenses/LICENSE-2.0
 163 ++ *
 164 ++ * Unless required by applicable law or agreed to in writing, software
 165 ++ * distributed under the License is distributed on an "AS IS" BASIS,
 166 ++ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
 167 ++ * See the License for the specific language governing permissions and
 168 ++ * limitations under the License.
 169 ++ */
 170 ++
 171 ++#include <gtest/gtest.h>
 172 ++
 173 ++#include <thrift/lib/cpp2/protocol/CompactProtocol.h>
 174 ++
 175 ++using namespace apache::thrift;
 176 ++
 177 ++TEST(ProtocolSkipTest, SkipInt) {
 178 ++  IOBufQueue queue;
 179 ++  CompactProtocolWriter writer;
 180 ++  writer.setOutput(&queue);
 181 ++  writer.writeI32(123);
 182 ++  auto buf = queue.move();
 183 ++  CompactProtocolReader reader;
 184 ++  reader.setInput(buf.get());
 185 ++  reader.skip(TType::T_I32);
 186 ++}
 187 ++
 188 ++TEST(ProtocolSkipTest, SkipStop) {
 189 ++  IOBufQueue queue;
 190 ++  CompactProtocolWriter writer;
 191 ++  writer.setOutput(&queue);
 192 ++  writer.writeFieldStop();
 193 ++  auto buf = queue.move();
 194 ++  CompactProtocolReader reader;
 195 ++  reader.setInput(buf.get());
 196 ++  bool thrown = false;
 197 ++  try {
 198 ++    reader.skip(TType::T_STOP);
 199 ++  } catch (const TProtocolException& ex) {
 200 ++    EXPECT_EQ(TProtocolException::INVALID_DATA, ex.getType());
 201 ++    thrown = true;
 202 ++  }
 203 ++  EXPECT_TRUE(thrown);
 204 ++}
 205 ++
 206 ++TEST(ProtocolSkipTest, SkipStopInContainer) {
 207 ++  IOBufQueue queue;
 208 ++  CompactProtocolWriter writer;
 209 ++  writer.setOutput(&queue);
 210 ++  writer.writeListBegin(TType::T_STOP, 1u << 30);
 211 ++  auto buf = queue.move();
 212 ++  CompactProtocolReader reader;
 213 ++  reader.setInput(buf.get());
 214 ++  bool thrown = false;
 215 ++  try {
 216 ++    reader.skip(TType::T_LIST);
 217 ++  } catch (const TProtocolException& ex) {
 218 ++    EXPECT_EQ(TProtocolException::INVALID_DATA, ex.getType());
 219 ++    thrown = true;
 220 ++  }
 221 ++  EXPECT_TRUE(thrown);
 222 ++}
 223 +--
 224 +2.13.5
 225 +
 226 --
 227 2.13.5
 228