Openwall GNU/*/Linux - a small security-enhanced Linux distro for servers
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Wed, 13 Nov 2013 22:03:36 +0100
From: Lukas Odzioba <lukas.odzioba@...il.com>
To: john-dev@...ts.openwall.com
Subject: Re: Fix broken formats (#420)

2013/11/11 magnum <john.magnum@...hmail.com>:
> I think we normally should not add that prefix. I reverted those. The
> original raw-md5 was for avoiding confusion with md5crypt when the latter
> was called just "md5".

O sure *crypt* stuff, now I remember, thanks.

Dhiru,
in keyring_fmt_plug and ocl version of the code there is field
"inlined" which seems to be unused. Is there any reason for keeping it
there?

Patch for keyring is attached,
Lukas

diff -urpN JohnTheRipper-bleeding-jumbo.orig//src/broken/opencl_keyring_fmt.c JohnTheRipper-bleeding-jumbo/src/broken/opencl_keyring_fmt.c
--- JohnTheRipper-bleeding-jumbo.orig//src/broken/opencl_keyring_fmt.c	2013-11-13 19:16:02.000000000 +0000
+++ JohnTheRipper-bleeding-jumbo/src/broken/opencl_keyring_fmt.c	2013-11-13 20:06:32.000000000 +0000
@@ -173,6 +173,17 @@ static void init(struct fmt_main *self)
 		fprintf(stderr, "Local worksize (LWS) %d, Global worksize (GWS) %d\n", (int)local_work_size, (int)global_work_size);
 }
 
+static int looks_like_nice_int(char *p)
+{
+	// reasonability check + avoids atoi's UB
+	if (strlen(p) > 9)
+		return 0;
+	for (; *p; p++)
+		if (*p < '0' || *p > '9')
+			return 0;
+	return 1;
+}
+
 static int valid(char *ciphertext, struct fmt_main *self)
 {
 	char *ctcopy, *keeptr, *p;
@@ -181,31 +192,45 @@ static int valid(char *ciphertext, struc
 		return 0;
 	ctcopy = strdup(ciphertext);
 	keeptr = ctcopy;
+	if (keeptr == NULL)
+		goto err;
 	ctcopy += 9;
 	if ((p = strtok(ctcopy, "*")) == NULL)	/* salt */
 		goto err;
-	if(strlen(p) != SALTLEN * 2)
+	if (strlen(p) != SALTLEN * 2)
 		goto err;
+	while (*p)
+		if (atoi16[ARCH_INDEX(*p++)] == 0x7f)
+			goto err;
 	if ((p = strtok(NULL, "*")) == NULL)	/* iterations */
 		goto err;
+	if (!looks_like_nice_int(p))
+		goto err;
 	if ((p = strtok(NULL, "*")) == NULL)	/* crypto size */
 		goto err;
+	if (!looks_like_nice_int(p))
+		goto err;
 	ctlen = atoi(p);
-	if ((p = strtok(NULL, "*")) == NULL)	/* inlined */
+	if ((p = strtok(NULL, "*")) == NULL)	/* inlined - unused? TODO */
+		goto err;
+	if (!looks_like_nice_int(p))
 		goto err;
 	if ((p = strtok(NULL, "*")) == NULL)	/* ciphertext */
 		goto err;
 	if (ctlen > LINE_BUFFER_SIZE)
 		goto err;
-	if(strlen(p) != ctlen * 2)
+	if (strlen(p) != ctlen * 2)
 		goto err;
-	if (strlen(p) < 32) /* this shouldn't happen for valid hashes */
+	if (strlen(p) < 32)	/* this shouldn't happen for valid hashes */
 		goto err;
+	while (*p)
+		if (atoi16[ARCH_INDEX(*p++)] == 0x7f)
+			goto err;
 
 	MEM_FREE(keeptr);
 	return 1;
 
-err:
+      err:
 	MEM_FREE(keeptr);
 	return 0;
 }
diff -urpN JohnTheRipper-bleeding-jumbo.orig//src/keyring_fmt_plug.c JohnTheRipper-bleeding-jumbo/src/keyring_fmt_plug.c
--- JohnTheRipper-bleeding-jumbo.orig//src/keyring_fmt_plug.c	2013-11-13 19:16:02.000000000 +0000
+++ JohnTheRipper-bleeding-jumbo/src/keyring_fmt_plug.c	2013-11-13 20:04:22.000000000 +0000
@@ -80,6 +80,17 @@ static void init(struct fmt_main *self)
 	cracked = mem_calloc_tiny(cracked_size, MEM_ALIGN_WORD);
 }
 
+static int looks_like_nice_int(char *p)
+{
+	// reasonability check + avoids atoi's UB
+	if (strlen(p) > 9)
+		return 0;
+	for (; *p; p++)
+		if (*p < '0' || *p > '9')
+			return 0;
+	return 1;
+}
+
 static int valid(char *ciphertext, struct fmt_main *self)
 {
 	char *ctcopy, *keeptr, *p;
@@ -88,31 +99,45 @@ static int valid(char *ciphertext, struc
 		return 0;
 	ctcopy = strdup(ciphertext);
 	keeptr = ctcopy;
+	if (keeptr == NULL)
+		goto err;
 	ctcopy += 9;
 	if ((p = strtok(ctcopy, "*")) == NULL)	/* salt */
 		goto err;
-	if(strlen(p) != SALTLEN * 2)
+	if (strlen(p) != SALTLEN * 2)
 		goto err;
+	while (*p)
+		if (atoi16[ARCH_INDEX(*p++)] == 0x7f)
+			goto err;
 	if ((p = strtok(NULL, "*")) == NULL)	/* iterations */
 		goto err;
+	if (!looks_like_nice_int(p))
+		goto err;
 	if ((p = strtok(NULL, "*")) == NULL)	/* crypto size */
 		goto err;
+	if (!looks_like_nice_int(p))
+		goto err;
 	ctlen = atoi(p);
-	if ((p = strtok(NULL, "*")) == NULL)	/* inlined */
+	if ((p = strtok(NULL, "*")) == NULL)	/* inlined - unused? TODO */
+		goto err;
+	if (!looks_like_nice_int(p))
 		goto err;
 	if ((p = strtok(NULL, "*")) == NULL)	/* ciphertext */
 		goto err;
 	if (ctlen > LINE_BUFFER_SIZE)
 		goto err;
-	if(strlen(p) != ctlen * 2)
+	if (strlen(p) != ctlen * 2)
 		goto err;
-	if (strlen(p) < 32) /* this shouldn't happen for valid hashes */
+	if (strlen(p) < 32)	/* this shouldn't happen for valid hashes */
 		goto err;
+	while (*p)
+		if (atoi16[ARCH_INDEX(*p++)] == 0x7f)
+			goto err;
 
 	MEM_FREE(keeptr);
 	return 1;
 
-err:
+      err:
 	MEM_FREE(keeptr);
 	return 0;
 }

Powered by blists - more mailing lists

Your e-mail address:

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