[OTR-dev] [PATCH] Don't crash when parsing arbitrary string with otrl_proto_query_bestversion()

[Conrad Hoffmann]

Hi folks,

first things first: a big thank you to all you who have devote time to
this awesome library - you rock!

But now enough sweet-talkin' ;)

I noticed that otrl_proto_query_bestversion() crashes when used on
arbitrary text that does not contain an OTR query. I admit that that's
obviously not the purpose of this function, but since it is
 a) a somewhat high-levelish, easy-to-use-looking interface
 b) not exactly thoroughly documented ;) and
 c) a possible attack vector
I figured it might be worth the time to avoid at least the segfault.

Here is the current code (comments by me):

  otrtag = strstr(otrquerymsg, "?OTR"); /* May return NULL! */
  otrtag += 4;

  /* First check is pointless, since otrtag was just incremented by 4 */
  if (otrtag && *otrtag == '?') {

So if no tag is found, strstr() returns NULL and then the memory address
0x4 is fetched, which just might segfault... a lot.

Attached patch moves the check right after the call to strstr() and gets
rid other checks since otrtag can't be NULL after the incrementation
anymore. Or did I miss anything?

I just subscribed to the list, so reply-to-list is sufficient.

Cheers and happy hacking,
Conrad
-------------- next part --------------
A non-text attachment was scrubbed...
Name: query_bestversion.patch
Type: text/x-patch
Size: 566 bytes
Desc: not available
URL: <http://lists.cypherpunks.ca/pipermail/otr-dev/attachments/20131012/15659398/attachment.bin>