[
https://issues.apache.org/jira/browse/THRIFT-1182?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13041419#comment-13041419
]
Ilya Maykov edited comment on THRIFT-1182 at 5/31/11 4:56 AM:
--------------------------------------------------------------
The original patch didn't call read\_(list|map|set)\_end if the contents were
skipped, which is incorrect. It happens to work for Binary and Compact
protocols because the read\_(list|map|set)\_methods are no-ops, but would fail
with a future implementation of JSON protocol, for example.
was (Author: ilyam):
The original patch didn't call read_(list|map|set)_end if the contents were
skipped, which is incorrect. It happens to work for Binary and Compact
protocols because the read_(list|map|set)_methods are no-ops, but would fail
with a future implementation of JSON protocol, for example.
> Native deserializer segfaults on incorrect list element type
> ------------------------------------------------------------
>
> Key: THRIFT-1182
> URL: https://issues.apache.org/jira/browse/THRIFT-1182
> Project: Thrift
> Issue Type: Bug
> Components: Ruby - Library
> Affects Versions: 0.6.1
> Environment: OS X 10.6 i686, Linux x86_64
> Reporter: Ilya Maykov
> Attachments: patch-THRIFT-1182-2.txt, patch-THRIFT-1182.txt,
> thrift_crash.rb
>
>
> There is a pretty major bug in the native Ruby deserializer that causes it to
> segfault on certain bad inputs. Basically when deserializing a list that is
> expected to contain elements of a basic type, but is claiming in the list
> header to be a list of structs, the native deserializer segfaults and crashes
> the ruby process. I will be attaching code that reproduces this shortly.
> I need to have a fix for this ASAP, and am willing to work on it, however I
> need some guidance, as I'm not sure what the desired behavior is. Basically
> the case is:
> * Expecting a list<string> (or list<i32>, list<i16>, etc.)
> * Get something that claims to be a list<struct>
> This can be caused by a buggy and/or malicious client, or by accidentally
> making a backwards-incompatible change to a .thrift definition and running
> both versions concurrently. Regardless of the cause, crashing on arbitrary
> inputs is not an option for my use case so I have to handle it somehow. I see
> 2 possible solutions:
> 1) Raise some kind of Type Mismatch error and catch it higher up, thus
> aborting parsing of the entire thrift struct
> 2) Skip the list with mismatched type but attempt to parse the rest of the
> struct.
> Of course, if the list header is wrong about the element type it could be
> wrong about the list length as well so it's not clear how many bytes should
> be skipped. Basically once such a type error is detected, the contents of the
> serialized struct can no longer be trusted. For this reason, I would lean
> towards option 1.
> Right now it doesn't look like the deserializer does much type checking while
> deserializing, so such a change in behavior could break numerous existing
> users and should probably be optional (i.e. right now you could have a
> list<i64> declared but a list<i32> come across the wire, I *think* the native
> deserializer will just read the list of i32s and happily convert them to Ruby
> Fixnums so everything will work. Adding the type checks I suggest would break
> this case.)
--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira
