You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@thrift.apache.org by je...@apache.org on 2020/02/08 22:31:16 UTC
[thrift] 01/01: THRIFT-5075: Backport changes for CVE-2019-0205 to
0.9.3.1 branch
This is an automated email from the ASF dual-hosted git repository.
jensg pushed a commit to branch 0.9.3.2
in repository https://gitbox.apache.org/repos/asf/thrift.git
commit 33dcbd08c080d589c639275b151e0b6ff0e6a0be
Author: Jens Geyer <je...@apache.org>
AuthorDate: Sat Feb 9 11:50:03 2019 +0100
THRIFT-5075: Backport changes for CVE-2019-0205 to 0.9.3.1 branch
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
---
.../org/apache/thrift/protocol/TProtocolUtil.as | 2 +-
.../src/thrift/c_glib/protocol/thrift_protocol.c | 59 +++++++++++++++-------
lib/cpp/src/thrift/protocol/TProtocol.h | 10 ++--
lib/d/src/thrift/protocol/base.d | 8 +--
lib/go/thrift/protocol.go | 2 -
.../org/apache/thrift/protocol/TProtocolUtil.java | 3 +-
lib/js/src/thrift.js | 3 --
lib/lua/TProtocol.lua | 8 +--
lib/nodejs/lib/thrift/binary_protocol.js | 2 -
lib/nodejs/lib/thrift/compact_protocol.js | 2 -
lib/ocaml/src/Thrift.ml | 3 +-
lib/py/src/protocol/TProtocol.py | 8 +--
lib/rb/lib/thrift/protocol/base_protocol.rb | 4 +-
lib/rb/spec/base_protocol_spec.rb | 1 -
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 513df95..22877b7 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 d6315d8..b73cfb5 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 0db2216..7a228a0 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 70648b3..5b6d845 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 45fa202..8d3f6a4 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 9bf10f6..c327448 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 955c2de..5297d3b 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 616e167..1306fb3 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 a230291..361f3df 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 8aee808..7649dbb 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 f0d7a42..063459b 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 311a635..88dc197 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 88f44d4..8cf5b98 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 ec50c48..7915791 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