Openwall GNU/*/Linux - a small security-enhanced Linux distro for servers
[<prev] [next>] [thread-next>] [day] [month] [year] [list]
Date: Mon, 23 Mar 2015 02:21:27 +0300
From: Alexander Cherepanov <ch3root@...nwall.com>
To: john-dev@...ts.openwall.com
Subject: Lengths of fields recorded in hashes

Hi!

For some formats, hashes contain variable-length data (say, encoded in 
hex) and the length of this data is also recorded in the hashes. One of 
the examples is 7z format which contains 3 data field (salt, iv, data) 
and 3 lengths for them.
I think their inclusion was an error and they should be removed. Mainly 
because they are useless and only complicate things.

Does anybody consider them useful? They could potentially be useful when 
one needs to allocate memory for the data but even in this case they 
save only one strlen (or equivalent).
OTOH each superfluous field now requires several lines of code in 
valid() and get_salt(). And parsing numbers is not that easy, e.g. atoi 
exhibits undefined behavior on overflows. Extra fields also take extra 
space etc. Recently the point was raised that lengths are an obstacle to 
fuzzing. And it would be nice if most of the hashes are constructed from 
a small number of simple blocks (a field containing a length of another 
field is not one of them).

I propose the following plan:
1) gradually rewrite valid()s and get_salt()s to ignore lengths recorded 
in hashes;
2) change the format of hashes in john and in 2john tools at some later 
point.

Can we change format of such hashes at will (by requiring to always use 
john and 2john tools of matching versions)?

Attached is a patch to 7z format which implements the first part of the 
plan as an illustration of the approach. (It also fixes a potential 
overflow in data field and removes strange dealing with zeroes in iv.)

WDYT?

-- 
Alexander Cherepanov

>From d024dc4c21f7d39b340137cb409d8b0f06496126 Mon Sep 17 00:00:00 2001
From: Alexander Cherepanov <ch3root@...nwall.com>
Date: Mon, 23 Mar 2015 02:19:56 +0300
Subject: [PATCH] 7z: compute data lengths from data fields in hash and bound
 them

---
 src/7z_fmt_plug.c |   34 ++++++++++++++++++----------------
 1 file changed, 18 insertions(+), 16 deletions(-)

diff --git a/src/7z_fmt_plug.c b/src/7z_fmt_plug.c
index 78f7a4f..690759d 100644
--- a/src/7z_fmt_plug.c
+++ b/src/7z_fmt_plug.c
@@ -121,23 +121,19 @@ static int valid(char *ciphertext, struct fmt_main *self)
 		goto err;
 	if ((p = strtokm(NULL, "$")) == NULL) /* salt length */
 		goto err;
-	len = atoi(p);
-	if(len > 16 || len < 0) /* salt length */
-		goto err;
+	/* ignore salt length recorded in the hash */
 	if ((p = strtokm(NULL, "$")) == NULL) /* salt */
 		goto err;
+	/* ignore salt */
 	if ((p = strtokm(NULL, "$")) == NULL) /* iv length */
 		goto err;
-	if (strlen(p) > 2)
-		goto err;
-	len = atoi(p);
-	if(len < 0 || len > 16) /* iv length */
-		goto err;
+	/* ignore iv length recorded in the hash */
 	if ((p = strtokm(NULL, "$")) == NULL) /* iv */
 		goto err;
 	if (!ishex(p))
 		goto err;
-	if (strlen(p) > len*2 && strcmp(p+len*2, "0000000000000000"))
+	len = strlen(p);
+	if(len > 16 * 2 || len % 2 != 0) /* iv length */
 		goto err;
 	if ((p = strtokm(NULL, "$")) == NULL) /* crc */
 		goto err;
@@ -145,14 +141,17 @@ static int valid(char *ciphertext, struct fmt_main *self)
 		goto err;
 	if ((p = strtokm(NULL, "$")) == NULL) /* data length */
 		goto err;
-	len = atoi(p);
+	/* ignore data length recorded in the hash */
 	if ((p = strtokm(NULL, "$")) == NULL) /* unpacksize */
 		goto err;
 	if (!isdec(p))	/* no way to validate, other than atoi() works for it */
 		goto err;
 	if ((p = strtokm(NULL, "$")) == NULL) /* data */
 		goto err;
-	if (strlen(p) != len * 2)	/* validates data_len atoi() */
+	if (!ishex(p))
+		goto err;
+	len = strlen(p);
+	if (len > BIG_ENOUGH * 2 || len % 2 != 0) /* data length */
 		goto err;
 
 	MEM_FREE(keeptr);
@@ -184,24 +183,27 @@ static void *get_salt(char *ciphertext)
 	p = strtokm(NULL, "$");
 	cs->NumCyclesPower = atoi(p);
 	p = strtokm(NULL, "$");
-	cs->SaltSize = atoi(p);
+	/* ignore salt length recorded in the hash */
 	p = strtokm(NULL, "$"); /* salt */
+	/* ignore salt */
 	p = strtokm(NULL, "$");
-	cs->ivSize = atoi(p);
+	/* ignore iv length recorded in the hash */
 	p = strtokm(NULL, "$"); /* iv */
-	for (i = 0; i < cs->ivSize; i++)
+	for (i = 0; p[i * 2]; i++)
 		cs->iv[i] = atoi16[ARCH_INDEX(p[i * 2])] * 16
 			+ atoi16[ARCH_INDEX(p[i * 2 + 1])];
+	cs->ivSize = i;
 	p = strtokm(NULL, "$"); /* crc */
 	cs->crc = atou(p); /* unsigned function */
 	p = strtokm(NULL, "$");
-	cs->length = atoi(p);
+	/* ignore data length recorded in the hash */
 	p = strtokm(NULL, "$");
 	cs->unpacksize = atoi(p);
 	p = strtokm(NULL, "$"); /* crc */
-	for (i = 0; i < cs->length; i++)
+	for (i = 0; p[i * 2]; i++)
 		cs->data[i] = atoi16[ARCH_INDEX(p[i * 2])] * 16
 			+ atoi16[ARCH_INDEX(p[i * 2 + 1])];
+	cs->length = i;
 	MEM_FREE(keeptr);
 	return (void *)cs;
 }
-- 
1.7.10.4


Powered by blists - more mailing lists

Your e-mail address:

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