Here are some issues about Thrift using TBinaryProtocol over TSocket -
they may or may not arise in other methods.
1. It is possible for a client to send an unbounded size message to the
server. Even if the server is only expecting scalar values, a client
could send an extra argument, and use it to contain an arbitrary sized
string, which would then have to get skipped.
2. If the server has RPCs with a container class or a string argument,
then the client can send an arbitrary sized argument (say an EXTREMELY
long string), which will then cause the program to allocate extermely
large amouts of memory, with the attendant consequences.
3. A client can leave a server hanging by sending an incomplete message
and never sending the remainder. Depending on the way the server is
programmed, this can result in a complete stall of the server or just
result in OS/program resources being blocked.
These are perhaps fundamental to the philosophy of thrift. I would
advocate adding a message byte-count and a timeout period for the
socket. The byte-count takes care of problems 1&2 - if the message is
too long, abort the message processing, and close the socket (possibly
after sending it a E2BIG equivalent T_EXCEPTION). The timeout period
takes care of hangs; if the timeout period expires, abort the message
processing and close the socket.
Other problems are related to the interface to the user
4. An RPC call will get executed even if not all the arguments are
satisfied. Thus, if the client calls foo(1: arg1, 2: arg2) and the
server is expecting foo(1: arg1, 2: arg2, 3:arg3), the server will get
invoked with some None (or not isset) arguments.
5. The incomplete type problem persists can occur later, too - for
instance, arg2 may be a pointer to a list of structs with 3 fields, but
the client may have populated arg2 with a list of structs of _2_ fields.
Solving this problem is an exercise in checking that the function is
invoked with the proper number of arguments with the proper type
properties. However, it would make this kind of programming easier if,
for each argument, we could ask if:
- the argument was passed
- the passed type is a super set of the expected type (i.e. for
non-struct types, the type matches exactly, and for a struct, it is
allowed to have more fields, but not less than the expected struct).
There are also problems related to the actual implementation of Thrift -
that is not an area I feel competent to speak to.
Laurens Van Houtven wrote:
If nobody minds, could this discussion be public? I care about the answer
too, and I can't imagine I'm the only person considering exposing a Thrift
service. It doesn't appear Thrift implementations are necessarily
specifically tested about what happens when you feed them random junk (this
was a mailing list topic a while ago), which would of course be a problem if
you're using it as an external interface, where every request is a potential
attack.
tia
lvh
