[THRIFT-4024] Skip() should throw on unknown data types

• Type: Bug
• Status: Closed
• Priority: Major
• Resolution: Fixed
• Affects Version/s: 0.10.0
• Fix Version/s: 0.13.0
• Component/s: C# - Library, Delphi - Library, Go - Library, Haxe - Library, netcore - Library
• Assignee: James E. King III
• Reporter: Michael Antipin
• Created: 10/Jan/17 11:35
• Updated: 25/Jan/20 08:56
• Resolved: 17/Feb/19 17:08

Description

I'm using TBinaryProtocol and a simple transport that reads from a given byte array.

C# library contains the following code in TProtocolUtil.Skip(TProtocol prot, TType type):

case TType.List:
	TList list = prot.ReadListBegin();
	for (int i = 0; i < list.Count; i++) {
		Skip(prot, list.ElementType);
	}
	prot.ReadListEnd();
	break;

The type of elements is detected in ReadListBegin(), and, as Skip() does nothing for unknown types, the position in the binary remains the same until the for loop completes.

So, when you try to deserialize invalid data, and a field type happens to be detected as TType.List, you may end up waiting for a random period of time until deserialization is completed (734707176 iterations of skipping in my case).

I suggest throwing an exception immediately when list elements type is unknown. May be, it would be good to have a setting like FailOnUnknownType, so that Skip() will throw instead of ignoring.

Issue Links

• Dependency: THRIFT-4783 Thrift should throw when skipping over unexpected data
• incorporates: THRIFT-4784 Thrift should throw when skipping over unexpected data
• is duplicated by: THRIFT-4997 Nexus Scan Reporting Security issue CVE-2019-0205 for Thrift:
• is related to: THRIFT-5075 Backport fixes for CVE-2019-0205 to (Java) 0.9.3-1 version
• links to: GitHub Pull Request #1155
• links to: GitHub Pull Request #1503
• links to: GitHub Pull Request #1742

Sub-Tasks

• Skip() should throw TProtocolException.INVALID_DATA on unknown data types: Closed
• Skip() should throw TProtocolException.INVALID_DATA on unknown data types: Closed
• Skip() should throw TProtocolException.INVALID_DATA on unknown data types: Closed
• Skip() should throw TProtocolException.INVALID_DATA on unknown data types: Closed
• Skip() should throw TProtocolException.INVALID_DATA on unknown data types: Closed
• Skip() should throw TProtocolException.INVALID_DATA on unknown data types: Closed
• Skip() should throw TProtocolException.INVALID_DATA on unknown data types: Closed
• Skip() should throw TProtocolException.INVALID_DATA on unknown data types: Closed
• Skip() should throw TProtocolException.INVALID_DATA on unknown data types: Closed
• Skip() should throw TProtocolException.INVALID_DATA on unknown data types: Resolved
• Skip() should throw TProtocolException.INVALID_DATA on unknown data types: Closed
• Skip() should throw TProtocolException.INVALID_DATA on unknown data types: Closed