Openwall GNU/*/Linux - a small security-enhanced Linux distro for servers
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Fri, 8 Jul 2016 16:19:20 +0300
From: Solar Designer <solar@...nwall.com>
To: john-dev@...ts.openwall.com
Subject: Re: rules.c bug/feature

On Fri, Jul 08, 2016 at 02:24:58PM +0200, magnum wrote:
> On 2016-07-06 18:27, Solar Designer wrote:
> >On Wed, Jul 06, 2016 at 05:27:33PM +0200, magnum wrote:
> >>While playing with some old contest rules I found a bug in John that's
> >>not Jumbo-specific: Apparently it lacks some checks so a 'ddd' rule will
> >>blow the destination buffer even at moderate input lengths (eg. 50).
> >
> >No, this shouldn't be the case.  It is assumed that any rule command may
> >double the word's length, and there's a safeguard inbetween commands.
> >
> >The buffers are:
> >
> >	char buffer[3][RULE_WORD_SIZE * 2 + CACHE_BANK_SHIFT];
> >
> >and the safeguard is:
> >
> >		in[RULE_WORD_SIZE - 1] = 0;
> >
> >Is this somehow broken?  We should identify the issue and fix it if so.
> 
> Sounds good, but then it must be broken somehow. The memcpy in 'd' did 
> blow the buffer and overwrote rules_data.classes and I verified this 
> happens in John proper too. I'm not sure why but I'll let you handle it.

You're right.  I didn't bother reproducing it, but I think I know what
the problem is: when I introduced the "length" variable some years ago,
I forgot to update the loop logic to clamp not only the buffer but also
this integer variable to the maximum length.  I think the attached patch
should fix it.  I'll test and commit it.

Alexander

Index: rules.c
===================================================================
RCS file: /home/cvs/cvsroot/Owl/packages/john/john/src/rules.c,v
retrieving revision 1.30
diff -u -r1.30 rules.c
--- rules.c	31 May 2016 07:11:51 -0000	1.30
+++ rules.c	8 Jul 2016 13:16:19 -0000
@@ -1,6 +1,6 @@
 /*
  * This file is part of John the Ripper password cracker,
- * Copyright (c) 1996-99,2003,2005,2009,2010,2015 by Solar Designer
+ * Copyright (c) 1996-99,2003,2005,2009,2010,2015,2016 by Solar Designer
  */
 
 #include <stdio.h>
@@ -349,7 +349,7 @@
 		in = buffer[2];
 
 	length = 0;
-	while (length < RULE_WORD_SIZE - 1) {
+	while (length < RULE_WORD_SIZE) {
 		if (!(in[length] = word[length]))
 			break;
 		length++;
@@ -378,7 +378,8 @@
 	which = 0;
 
 	while (RULE) {
-		in[RULE_WORD_SIZE - 1] = 0;
+		if (length >= RULE_WORD_SIZE)
+			in[length = RULE_WORD_SIZE - 1] = 0;
 
 		switch (LAST) {
 /* Crack 4.1 rules */

Powered by blists - more mailing lists

Your e-mail address:

Powered by Openwall GNU/*/Linux - Powered by OpenVZ