Invalid data in remote protocol can cause buffer overflow

[Richard Boulton]

The implementation of the remote protocol is vulnerable to buffer overflows. In particular, the code in remoteserver.cc which decodes messages does not check that lengths of objects read from the stream of data do not overflow: for example, at line 266 of remoteserver.cc, we have:

len = decode_length(&p, p_end); AutoPtr<Xapian::Query::Internal>

query(Xapian::Query::Internal::unserialise(string(p, len)));

But len might be longer than the remaining length of the input message, pointed to by p.

Marking this a minor severity because the communication channel is not intended to be exposed to untrusted systems. However, it would be good to fix this when possible to improve robustness.

[Olly Betts]

Hmm. At least it's a "read-only" buffer-overflow, so the risks are denial of service (because it could SEGV or throw bad_alloc, or possibly be caused to do lots of work for a small amount of network traffic (though that's almost trivially true for a search server anyway!)) or information leak (by reading potentially sensitive data which happens to lie beyond the end of the buffer, and getting it decoded in such a way that it can be read - Xapian itself has few secrets, but the application using it might).

I think we should fix this for 1.0. It should just be a matter of looking at any call to decode_length() and if it's used as below then add a check. I'll take a look.

[Richard Boulton]

I'm going to be looking at the remote protocol code in detail very shortly to implement the postlist stuff - if you've looked at this already, let me know, otherwise, I'll take a look while I'm there. I'd think the neatest fix is just to define a helper function which safely decodes a string from the input, performing bounds checking on the length read from the input.

[Olly Betts]

I've actually already implemented a fix. If bugzilla stops sending me mail, I might even manage to check it in soon!

[Richard Boulton]

The fix that Olly checked in looks good, and passes the testsuite now that I've fixed the test, so I'm closing this bug. The remote protocol still shouldn't be considered secure until it's been audited (there's a job if anyone wants one), but this particular problem with it is resolved.

[Olly Betts]

Fixed in 1.0.0 release.