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
diff --git a/src/proto.c b/src/proto.c
index dba32e2..8d89027 100644
--- a/src/proto.c
+++ b/src/proto.c
@@ -294,13 +294,16 @@ unsigned int otrl_proto_query_bestversion(const char *otrquerymsg,
otrtag = strstr(otrquerymsg, "?OTR");
+ if (!otrtag) {
+ return 0;
+ }
otrtag += 4;
- if (otrtag && *otrtag == '?') {
+ if (*otrtag == '?') {
query_versions = (1<<0);
++otrtag;
}
- if (otrtag && *otrtag == 'v') {
+ if (*otrtag == 'v') {
for(++otrtag; *otrtag && *otrtag != '?'; ++otrtag) {
switch(*otrtag) {
case '2':
_______________________________________________
OTR-dev mailing list
[email protected]
http://lists.cypherpunks.ca/mailman/listinfo/otr-dev
