Bug#337127: gaim-encryption patch: check sscanf() return value

[Max Kellermann]

On 2006/09/17 05:20, Bill Tompkins [EMAIL PROTECTED] wrote:
 Actually, I believe the bug is simply that 'realstart' is not
 assigned at the beginning of the function.  If it is set to zero,
 the check correctly detects that it wasn't assigned, and things are
 (moderately) happy:

I also considered this smaller patch, but dismissed it because the
error message would be something like can't decrypt instead of the
more correct no colon found - but you're the upstream author, you're
the one to decide..

Max



-- 
To UNSUBSCRIBE, email to [EMAIL PROTECTED]
with a subject of unsubscribe. Trouble? Contact [EMAIL PROTECTED]

Bug#337127: gaim-encryption patch: check sscanf() return value

[Max Kellermann]

The bug is caused by a wrong sscanf() return value check.  The
sscanf() is called with two escapes, but the caller only checks
whether one of those is valid.  This patch expands the check to both
values.

--- keys.c.orig 2006-09-16 20:07:30.137499500 +0200
+++ keys.c  2006-09-16 20:07:35.621842250 +0200
@@ -211,7 +211,7 @@
   return;
}
 
-   if ( (sscanf(key_len_msg, : Len %u:%n, length, realstart)  1) ||
+   if ( (sscanf(key_len_msg, : Len %u:%n, length, realstart)  2) ||
 (realstart == 0) ) {
   gaim_debug(GAIM_DEBUG_ERROR, gaim-encryption, Error in key header\n);
   return;

Bug#337127: gaim-encryption patch: check sscanf() return value

[Max Kellermann]

On 2006/09/16 20:11, Max Kellermann [EMAIL PROTECTED] wrote:
 The bug is caused by a wrong sscanf() return value check.  The
 sscanf() is called with two escapes, but the caller only checks
 whether one of those is valid.  This patch expands the check to both
 values.

I was wrong, since '%n' does not increment sscanf's return value.
Unfortunately, there is no way for the sscanf caller to see the
difference between everything was parsed fine and no colon
detected, the variable 'realstart' wasn't assigned.

I changed the patch and removed the colon from the sscanf format
string.  The colon check is performed manually.

--- keys.c.orig 2006-09-16 20:37:43.474826000 +0200
+++ keys.c  2006-09-16 20:32:56.464889000 +0200
@@ -211,13 +211,20 @@
   return;
}
 
-   if ( (sscanf(key_len_msg, : Len %u:%n, length, realstart)  1) ||
+   if ( (sscanf(key_len_msg, : Len %u%n, length, realstart)  1) ||
 (realstart == 0) ) {
   gaim_debug(GAIM_DEBUG_ERROR, gaim-encryption, Error in key header\n);
   return;
}
 
key_len_msg += realstart;
+   if (key_len_msg[0] != ':') {
+  gaim_debug(GAIM_DEBUG_ERROR, gaim-encryption, Colon expected\n);
+  return;
+   }
+
+   ++key_len_msg;
+
if (strlen(key_len_msg)  length) {
   gaim_debug(GAIM_DEBUG_ERROR, gaim-encryption, Length doesn't match in 
add_key\n);
   return;

Bug#337127: gaim-encryption patch: check sscanf() return value

[Bill Tompkins]

Actually, I believe the bug is simply that 'realstart' is not assigned
at the beginning of the function.  If it is set to zero, the check
correctly detects that it wasn't assigned, and things are (moderately)
happy:

--- gaim-encryption-2.38/keys.c 2005-06-11 13:40:33.0 -0400
+++ gaim-encryption-2.39/keys.c 2006-09-16 23:10:07.0 -0400
@@ -184,7 +184,7 @@
crypt_proto* proto=0;
unsigned char* key_len_msg=0;
unsigned int length;
-   int realstart;
+   int realstart=0;
gchar** after_key;
gchar* resend_msg_id = 0;

Fixing this, and firing that test message again, the code will hit a bug
that was already fixed in the 3.0 branch, which will cause the plugin to
hang.  So... I'm releasing a 2.39 tonight with a complete fix, and a few
other fixes backported from 3.0, and a few memory leak fixes just
reported today (Hi Max!).

Thanks all for the diligence reporting this and tracking it down,

-Bill




-- 
To UNSUBSCRIBE, email to [EMAIL PROTECTED]
with a subject of unsubscribe. Trouble? Contact [EMAIL PROTECTED]