THRIFT-5075: Backport changes for CVE-2019-0205 to 0.9.3.1 branch0.9.3.2

This closes #1993

THRIFT-4024, THRIFT-4783, THRIFT-4784: throw when skipping invalid type (#1742)
* THRIFT-4024: make c_glib throw on unsupported type when skipping
* THRIFT-4783: throw on invalid skip (py)
* THRIFT-4024: make cpp throw on unsupported type when skipping
* THRIFT-4024: uniform skip behavior on unsupported type
* THRIFT-4784: Thrift should throw when skipping over unexpected data

14 files changed, 66 insertions, 49 deletions

diff --git a/lib/as3/src/org/apache/thrift/protocol/TProtocolUtil.as b/lib/as3/src/org/apache/thrift/protocol/TProtocolUtil.as
index 513df954b..22877b75b 100644
--- a/lib/as3/src/org/apache/thrift/protocol/TProtocolUtil.as
+++ b/lib/as3/src/org/apache/thrift/protocol/TProtocolUtil.as
@@ -141,7 +141,7 @@ package org.apache.thrift.protocol {
           break;
         }
         default:
-          break;
+          throw new TProtocolError(TProtocolError.INVALID_DATA, "invalid data");
       }
     }
   }
diff --git a/lib/c_glib/src/thrift/c_glib/protocol/thrift_protocol.c b/lib/c_glib/src/thrift/c_glib/protocol/thrift_protocol.c
index d6315d8fb..b73cfb591 100644
--- a/lib/c_glib/src/thrift/c_glib/protocol/thrift_protocol.c
+++ b/lib/c_glib/src/thrift/c_glib/protocol/thrift_protocol.c
@@ -419,6 +419,13 @@ thrift_protocol_read_binary (ThriftProtocol *protocol, gpointer *buf,
                                                             len, error);
 }
 
+#define THRIFT_SKIP_RESULT_OR_RETURN(_RES, _CALL) \
+  { \
+    gint32 _x = (_CALL); \
+    if (_x < 0) { return _x; } \
+    (_RES) += _x; \
+  }
+
 gint32
 thrift_protocol_skip (ThriftProtocol *protocol, ThriftType type, GError **error)
 {
@@ -469,34 +476,44 @@ thrift_protocol_skip (ThriftProtocol *protocol, ThriftType type, GError **error)
         gchar *name;
         gint16 fid;
         ThriftType ftype;
-        result += thrift_protocol_read_struct_begin (protocol, &name, error);
-
+        THRIFT_SKIP_RESULT_OR_RETURN(result,
+          thrift_protocol_read_struct_begin (protocol, &name, error))
         while (1)
         {
-          result += thrift_protocol_read_field_begin (protocol, &name, &ftype,
-                                                      &fid, error);
+          THRIFT_SKIP_RESULT_OR_RETURN(result,
+            thrift_protocol_read_field_begin (protocol, &name, &ftype,
+                                              &fid, error))
           if (ftype == T_STOP)
           {
             break;
           }
-          result += thrift_protocol_skip (protocol, ftype, error);
-          result += thrift_protocol_read_field_end (protocol, error);
+          THRIFT_SKIP_RESULT_OR_RETURN(result,
+            thrift_protocol_skip (protocol, ftype, error))
+          THRIFT_SKIP_RESULT_OR_RETURN(result,
+            thrift_protocol_read_field_end (protocol, error))
         }
-        result += thrift_protocol_read_struct_end (protocol, error);
+        THRIFT_SKIP_RESULT_OR_RETURN(result,
+          thrift_protocol_read_struct_end (protocol, error))
         return result;
       }
     case T_MAP:
       {
-        guint32 result = 0;
+        gint32 result = 0;
         ThriftType elem_type;
+        ThriftType key_type;
         guint32 i, size;
-        result += thrift_protocol_read_set_begin (protocol, &elem_type, &size,
-                                                  error);
+        THRIFT_SKIP_RESULT_OR_RETURN(result,
+          thrift_protocol_read_map_begin (protocol, &key_type, &elem_type, &size,
+                                          error))
         for (i = 0; i < size; i++)
         {
-          result += thrift_protocol_skip (protocol, elem_type, error);
+          THRIFT_SKIP_RESULT_OR_RETURN(result,
+            thrift_protocol_skip (protocol, key_type, error))
+          THRIFT_SKIP_RESULT_OR_RETURN(result,
+            thrift_protocol_skip (protocol, elem_type, error))
         }
-        result += thrift_protocol_read_set_end (protocol, error);
+        THRIFT_SKIP_RESULT_OR_RETURN(result,
+          thrift_protocol_read_map_end (protocol, error))
         return result;
       }
     case T_LIST:
@@ -504,18 +521,26 @@ thrift_protocol_skip (ThriftProtocol *protocol, ThriftType type, GError **error)
         guint32 result = 0;
         ThriftType elem_type;
         guint32 i, size;
-        result += thrift_protocol_read_list_begin (protocol, &elem_type, &size,
-                                                   error);
+        THRIFT_SKIP_RESULT_OR_RETURN(result,
+          thrift_protocol_read_list_begin (protocol, &elem_type, &size,
+                                           error))
         for (i = 0; i < size; i++)
         {
-          result += thrift_protocol_skip (protocol, elem_type, error);
+          THRIFT_SKIP_RESULT_OR_RETURN(result,
+          thrift_protocol_skip (protocol, elem_type, error))
         }
-        result += thrift_protocol_read_list_end (protocol, error);
+        THRIFT_SKIP_RESULT_OR_RETURN(result,
+          thrift_protocol_read_list_end (protocol, error))
         return result;
       }
     default:
-      return 0;
+      break;
   }
+
+  g_set_error (error, THRIFT_PROTOCOL_ERROR,
+               THRIFT_PROTOCOL_ERROR_INVALID_DATA,
+               "unrecognized type");
+  return -1;
 }
 
 /* define the GError domain for Thrift protocols */
diff --git a/lib/cpp/src/thrift/protocol/TProtocol.h b/lib/cpp/src/thrift/protocol/TProtocol.h
index 0db221698..7a228a0a7 100644
--- a/lib/cpp/src/thrift/protocol/TProtocol.h
+++ b/lib/cpp/src/thrift/protocol/TProtocol.h
@@ -737,14 +737,12 @@ uint32_t skip(Protocol_& prot, TType type) {
     result += prot.readListEnd();
     return result;
   }
-  case T_STOP:
-  case T_VOID:
-  case T_U64:
-  case T_UTF8:
-  case T_UTF16:
+  default:
     break;
   }
-  return 0;
+
+  throw TProtocolException(TProtocolException::INVALID_DATA,
+                           "invalid TType");
 }
 
 }}} // apache::thrift::protocol
diff --git a/lib/d/src/thrift/protocol/base.d b/lib/d/src/thrift/protocol/base.d
index 70648b3c3..5b6d84514 100644
--- a/lib/d/src/thrift/protocol/base.d
+++ b/lib/d/src/thrift/protocol/base.d
@@ -260,7 +260,7 @@ protected:
  * in generated code.
  */
 void skip(Protocol)(Protocol prot, TType type) if (is(Protocol : TProtocol)) {
-  final switch (type) {
+  switch (type) {
     case TType.BOOL:
       prot.readBool();
       break;
@@ -324,9 +324,9 @@ void skip(Protocol)(Protocol prot, TType type) if (is(Protocol : TProtocol)) {
       }
       prot.readSetEnd();
       break;
-    case TType.STOP: goto case;
-    case TType.VOID:
-      assert(false, "Invalid field type passed.");
+
+    default:
+      throw new TProtocolException(TProtocolException.Type.INVALID_DATA);
   }
 }
 
diff --git a/lib/go/thrift/protocol.go b/lib/go/thrift/protocol.go
index 45fa202e7..8d3f6a41b 100644
--- a/lib/go/thrift/protocol.go
+++ b/lib/go/thrift/protocol.go
@@ -94,8 +94,6 @@ func Skip(self TProtocol, fieldType TType, maxDepth int) (err error) {
 	}
 
 	switch fieldType {
-	case STOP:
-		return
 	case BOOL:
 		_, err = self.ReadBool()
 		return
diff --git a/lib/javame/src/org/apache/thrift/protocol/TProtocolUtil.java b/lib/javame/src/org/apache/thrift/protocol/TProtocolUtil.java
index 9bf10f67e..c327448ef 100644
--- a/lib/javame/src/org/apache/thrift/protocol/TProtocolUtil.java
+++ b/lib/javame/src/org/apache/thrift/protocol/TProtocolUtil.java
@@ -152,7 +152,8 @@ public class TProtocolUtil {
         break;
       }
     default:
-      break;
+        throw new TProtocolException(TProtocolException.INVALID_DATA,
+                                   "Unrecognized type " + type);
     }
   }
 }
diff --git a/lib/js/src/thrift.js b/lib/js/src/thrift.js
index 955c2de21..5297d3b53 100644
--- a/lib/js/src/thrift.js
+++ b/lib/js/src/thrift.js
@@ -1317,9 +1317,6 @@ Thrift.Protocol.prototype = {
     skip: function(type) {
         var ret, i;
         switch (type) {
-            case Thrift.Type.STOP:
-                return null;
-
             case Thrift.Type.BOOL:
                 return this.readBool();
 
diff --git a/lib/lua/TProtocol.lua b/lib/lua/TProtocol.lua
index 616e167a9..1306fb3d8 100644
--- a/lib/lua/TProtocol.lua
+++ b/lib/lua/TProtocol.lua
@@ -107,9 +107,7 @@ function TProtocolBase:readDouble() end
 function TProtocolBase:readString() end
 
 function TProtocolBase:skip(ttype)
-  if type == TType.STOP then
-    return
-  elseif ttype == TType.BOOL then
+  if ttype == TType.BOOL then
     self:readBool()
   elseif ttype == TType.BYTE then
     self:readByte()
@@ -153,6 +151,10 @@ function TProtocolBase:skip(ttype)
       self:skip(ettype)
     end
     self:readListEnd()
+  else
+    terror(TProtocolException:new{
+      message = 'Invalid data'
+    })
   end
 end
 
diff --git a/lib/nodejs/lib/thrift/binary_protocol.js b/lib/nodejs/lib/thrift/binary_protocol.js
index a230291ec..361f3df18 100644
--- a/lib/nodejs/lib/thrift/binary_protocol.js
+++ b/lib/nodejs/lib/thrift/binary_protocol.js
@@ -293,8 +293,6 @@ TBinaryProtocol.prototype.getTransport = function() {
 
 TBinaryProtocol.prototype.skip = function(type) {
   switch (type) {
-    case Type.STOP:
-      return;
     case Type.BOOL:
       this.readBool();
       break;
diff --git a/lib/nodejs/lib/thrift/compact_protocol.js b/lib/nodejs/lib/thrift/compact_protocol.js
index 8aee808f5..7649dbb05 100644
--- a/lib/nodejs/lib/thrift/compact_protocol.js
+++ b/lib/nodejs/lib/thrift/compact_protocol.js
@@ -841,8 +841,6 @@ TCompactProtocol.prototype.zigzagToI64 = function(n) {
 
 TCompactProtocol.prototype.skip = function(type) {
   switch (type) {
-    case Type.STOP:
-      return;
     case Type.BOOL:
       this.readBool();
       break;
diff --git a/lib/ocaml/src/Thrift.ml b/lib/ocaml/src/Thrift.ml
index f0d7a4296..063459ba0 100644
--- a/lib/ocaml/src/Thrift.ml
+++ b/lib/ocaml/src/Thrift.ml
@@ -206,8 +206,6 @@ struct
         (* skippage *)
     method skip typ =
       match typ with
-        | T_STOP -> ()
-        | T_VOID -> ()
         | T_BOOL -> ignore self#readBool
         | T_BYTE
         | T_I08 -> ignore self#readByte
@@ -248,6 +246,7 @@ struct
                               self#readListEnd)
         | T_UTF8 -> ()
         | T_UTF16 -> ()
+        | _ -> raise (Protocol.E (Protocol.INVALID_DATA, "Invalid data"))
   end
 
   class virtual factory =
diff --git a/lib/py/src/protocol/TProtocol.py b/lib/py/src/protocol/TProtocol.py
index 311a635fa..88dc197e8 100644
--- a/lib/py/src/protocol/TProtocol.py
+++ b/lib/py/src/protocol/TProtocol.py
@@ -160,9 +160,7 @@ class TProtocolBase:
     pass
 
   def skip(self, ttype):
-    if ttype == TType.STOP:
-      return
-    elif ttype == TType.BOOL:
+    if ttype == TType.BOOL:
       self.readBool()
     elif ttype == TType.BYTE:
       self.readByte()
@@ -201,6 +199,10 @@ class TProtocolBase:
       for i in xrange(size):
         self.skip(etype)
       self.readListEnd()
+    else:
+      raise TProtocolException(
+          TProtocolException.INVALID_DATA,
+          "invalid TType")
 
   # tuple of: ( 'reader method' name, is_container bool, 'writer_method' name )
   _TTYPE_HANDLERS = (
diff --git a/lib/rb/lib/thrift/protocol/base_protocol.rb b/lib/rb/lib/thrift/protocol/base_protocol.rb
index 88f44d46d..8cf5b9846 100644
--- a/lib/rb/lib/thrift/protocol/base_protocol.rb
+++ b/lib/rb/lib/thrift/protocol/base_protocol.rb
@@ -323,8 +323,6 @@ module Thrift
 
     def skip(type)
       case type
-      when Types::STOP
-        nil
       when Types::BOOL
         read_bool
       when Types::BYTE
@@ -367,6 +365,8 @@ module Thrift
           skip(etype)
         end
         read_list_end
+      else
+        raise ProtocolException.new(ProtocolException::INVALID_DATA, 'Invalid data')
       end
     end
   end
diff --git a/lib/rb/spec/base_protocol_spec.rb b/lib/rb/spec/base_protocol_spec.rb
index ec50c4823..79157916e 100644
--- a/lib/rb/spec/base_protocol_spec.rb
+++ b/lib/rb/spec/base_protocol_spec.rb
@@ -158,7 +158,6 @@ describe 'BaseProtocol' do
       @prot.skip(Thrift::Types::I64)
       @prot.skip(Thrift::Types::DOUBLE)
       @prot.skip(Thrift::Types::STRING)
-      @prot.skip(Thrift::Types::STOP) # should do absolutely nothing
     end
 
     it "should skip structs" do