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:15 UTC
[thrift] branch 0.9.3.2 created (now 33dcbd0)
This is an automated email from the ASF dual-hosted git repository.
jensg pushed a change to branch 0.9.3.2
in repository https://gitbox.apache.org/repos/asf/thrift.git.
at 33dcbd0 THRIFT-5075: Backport changes for CVE-2019-0205 to 0.9.3.1 branch
This branch includes the following new commits:
new 33dcbd0 THRIFT-5075: Backport changes for CVE-2019-0205 to 0.9.3.1 branch
The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
[thrift] 01/01: THRIFT-5075: Backport changes for CVE-2019-0205 to
0.9.3.1 branch
Posted by je...@apache.org.
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