Follow @Openwall on Twitter for new release announcements and other news
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Sun, 23 Dec 2012 22:04:54 +0530
From: Dhiru Kholia <dhiru.kholia@...il.com>
To: john-dev@...ts.openwall.com
Subject: Re: scan-build results, part 1

On Sun, Dec 23, 2012 at 9:46 PM, magnum <john.magnum@...hmail.com> wrote:
> Another weird complaint is for MSCHAPv2. The pos pointer is set to non-null in line 429. How could it ever be a null dereference in line 433?

I have received an "official" answer on this one. It is *not* a false
positive. "ciphertext" can be NULL and we haven't checked for it
before doing pointer arithmetic on line 429.
gwynne> From the analyzer's point of view, NULL acts like NaN; i.e.
"NULL + anything = NULL" in terms of pointer validity.

Following patch makes this problem go away.

diff --git a/src/MSCHAPv2_fmt_plug.c b/src/MSCHAPv2_fmt_plug.c
index d946036..b4c7fcf 100644
--- a/src/MSCHAPv2_fmt_plug.c
+++ b/src/MSCHAPv2_fmt_plug.c
@@ -44,6 +44,7 @@

 #include "sha.h"
 #include <openssl/des.h>
+#include <assert.h>

 #ifndef uchar
 #define uchar unsigned char
@@ -426,6 +427,7 @@ static void *mschapv2_get_salt(char *ciphertext)
   SHA1_Init(&ctx);

   /* Peer Challenge */
+  assert (ciphertext != NULL);
   pos = ciphertext + 10 + 16*2 + 1 + 24*2 + 1; /* Skip $MSCHAPv2$,
Authenticator Challenge and Response Hash

   memset(tmp, 0, 16);


-- 
Cheers,
Dhiru

Powered by blists - more mailing lists

Confused about mailing lists and their use? Read about mailing lists on Wikipedia and check out these guidelines on proper formatting of your messages.