Perl protocol handler could be more robust against unrecognised types
---------------------------------------------------------------------
Key: THRIFT-840
URL: https://issues.apache.org/jira/browse/THRIFT-840
Project: Thrift
Issue Type: Bug
Components: Library (Perl)
Affects Versions: 0.3
Environment: Debian Linux 5.0 (stable), Perl 5.10.0.
Reporter: Conrad Hughes
Priority: Minor
Fix For: 0.3
The Perl protocol implementation can loop arbitrarily on corrupt data because
Protocol::skip() skips nothing if it doesn't recognise a type. It might be
nice if it threw an exception instead. For example, the loop to skip a map
(lib/perl/lib/Thrift/Protocol.pm:374) will keep looking at the same invalid
bytes for every iteration if it hits an unrecognised type. If corrupt data has
resulted in a bogus huge map size, then the loop goes on for a long while,
rather than dying quickly after running out of available data.
I recognise that input validation probably isn't a priority for Thrift (usually
communications are well-formed!), but wonder if you'd consider the attached
patch which dies if skip or skipBinary encounter types that they don't know
what to do with. It probably needs more thought than I've given it, as I'm not
sure what the correct behaviour should be for valid types not handled by the
existing code: in the patch I'm throwing an exception for those saying that
they cannot be skipped. An additional TType constant of MAX_TYPE might be
helpful in writing a better solution.
I know it's a bit weird to be suggesting this; I'm using Thrift for
serialisation among other things, and with an occasionally-imperfect data
store. This "infinite" loop was the only instance in which your existing error
handling didn't adequately flag up bad data. Clearly I should also be doing
checksums on the data beforehand, but I just thought I'd suggest this to you.
Thanks,
Conrad
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
