Openwall GNU/*/Linux - a small security-enhanced Linux distro for servers
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Tue, 21 Jul 2015 20:18:40 +0800
From: Kai Zhao <loverszhao@...il.com>
To: john-dev@...ts.openwall.com
Subject: Re: more robustness

Hi Alexander,

> Maybe you can even write a script that would spot some of the likely
> improper flag (non-)uses.  e.g. a _fmt*.c file mentions OpenMP stuff,
> but never mentions FMT_OMP, or vice versa.  Some of this could be easier
> detected at runtime - e.g., "\x20" and "\xa0" hashing differently, but a
> format lacks FMT_8_BIT, or vice versa.  Your builtin fuzzer or extended
> self-test could detect that.

I have written a script to detect the likely improper flag. The script is
attached. The script searches the whole file to find key words, such as
"_OPENMP", and FMT flags, such as "FMT_OMP".

If the source code file contains "_OPENMP", it should has    "FMT_OMP".
The same as "dyna_salt.h" and "FMT_DYNA_SALT", "DES_bs.h" and
"FMT_BS", "dynamic.h" and "FMT_DYNAMIC".

Run the script by: ./detect_improper_flags.pl  src/*fmt_plug.c

The warnings will be saved in fmt_flags_warning.log. However, most
of the warnings are not really problems. All the warnings should be
manually reviewed.

I found there are maybe some problems:

1. net_md5_fmt_plug.c and net_sha1_fmt_plug.c have included
"dynamic.h", but their flags do not contain FMT_DYNAMIC.

2. opencl_lm_fmt_plug.c has flag FMT_BS but it does not include
"DES_bs.h".

As to the FMT_SPLIT_UNIFIES_CASE flag, it is difficult to detect
by script file since it is related to strlwr, strupr, enc_strlwr,
enc_strupper and maybe other funcitons. So I changed the formats.c::
fmt_self_test_body() to only detect whether the
FMT_SPLIT_UNIFIES_CASE flag is wrong.

The idea is simple. Change the ciphertext to lower case before split(),
and the format changes case if the return of split has upper case.

diff --git a/src/formats.c b/src/formats.c
index 814b2db..25106f1 100644
--- a/src/formats.c
+++ b/src/formats.c
@@ -370,7 +370,94 @@ static char *fmt_self_test_body(struct fmt_main
*format,
                }
 #endif

-               ciphertext = format->methods.split(ciphertext, 0, format);
+               char *ciphertext_copy, *pcase, *out;
+               int flag_change_case = 0;
+
+               ciphertext_copy = strdup(ciphertext);
+               pcase = ciphertext_copy;
+
+               // Move pacase to the fist char which is not tag
+               if (*pcase == '#39;) {
+                       pcase++;
+                       while (*pcase && *pcase != '#39;)
+                               pcase++;
+                       if (*pcase == 0)
+                               goto pass;
+                       pcase++;
+               }
+
+               // Change ciphertext to upper case
+               strupr(pcase);
+               out = format->methods.split(ciphertext_copy, 0, format);
+               if (*out == '#39;) {
+                       out++;
+                       while (*out && *out != '#39;)
+                               out++;
+                       if (*out == 0)
+                               goto pass;
+                       out++;
+               }
+               while (*out && *pcase) {
+                       if (*out != *pcase) {
+                               flag_change_case = 1;
+                               break;
+                       }
+                       out++;
+                       pcase++;
+               }
+
+               if (flag_change_case == 1 && !(format->params.flags &
FMT_SPLIT_UNIFIES_CASE))
+                       goto wrong;
+
+               if (flag_change_case == 1 && (format->params.flags &
FMT_SPLIT_UNIFIES_CASE))
+                       goto pass;
+
+               // Change ciphertext to lower case
+               pcase = ciphertext_copy;
+
+               // Move pacase to the fist char which is not tag
+               if (*pcase == '#39;) {
+                       pcase++;
+                       while (*pcase && *pcase != '#39;)
+                               pcase++;
+                       if (*pcase == 0)
+                               goto pass;
+                       pcase++;
+               }
+
+               strlwr(pcase);
+               out = format->methods.split(ciphertext_copy, 0, format);
+               if (*out == '#39;) {
+                       out++;
+                       while (*out && *out != '#39;)
+                               out++;
+                       if (*out == 0)
+                               goto pass;
+                       out++;
+               }
+               while (*out && *pcase) {
+                       if (*out != *pcase) {
+                               flag_change_case = 1;
+                               break;
+                       }
+                       out++;
+                       pcase++;
+               }
+
+               if (flag_change_case == 1 && !(format->params.flags &
FMT_SPLIT_UNIFIES_CASE))
+                       goto wrong;
+
+               if (flag_change_case == 0 && (format->params.flags &
FMT_SPLIT_UNIFIES_CASE))
+                       goto wrong;
+
+pass:
+               MEM_FREE(ciphertext_copy);
+               return NULL;
+
+wrong:
+               MEM_FREE(ciphertext_copy);
+               return "FMT_* flags Wrong: FMT_SPLIT_UNIFIES_CASE";
+

$ make -sj8
$ ./john --test=0 > log

Finally, there are no formats have obvious problems with
FMT_SPLIT_UNIFIES_CASE flag. But there are 3 formats: MediaWiki,
PHPS, PHPS2 which do not contain the flag and their split do not
change case. But the 3 formats finally has the flag:
FMT_SPLIT_UNIFIES_CASE.

I think it is because they are dynamic formats, and their flags are
are changed at dynamic_fmt.c::7449
pFmt->params.flags |= FMT_SPLIT_UNIFIES_CASE;

I do't know whether the 3 formats should contain the split flag.


Thanks,

Kai

[ CONTENT OF TYPE text/html SKIPPED ]

#!/usr/bin/perl

$warning_nums = 0;

@key_words = ("_OPENMP", "dyna_salt\\.h", "DES_bs\\.h", "dynamic\\.h");
@has_key_worlds = ();
@flags = ("FMT_OMP", "FMT_DYNA_SALT", "FMT_BS", "FMT_DYNAMIC");
@has_flags = ();

if ($#key_words != $#flags) {
	print "key_words and flags must has the same length\n";
	exit(1);
}

for ($i = 0; $i <= $#key_words; $i++)  {
	$has_key_words[$i] = 0;
	$has_flags[$i] = 0;
}

foreach $argnum (0 .. $#ARGV) {
	# print "$ARGV[$argnum]\n";
	test_file($ARGV[$argnum]);
}

print "There are $warning_nums warnings.\n";

sub test_file {

	#print "Testing file:@...";

	# Clear array
	for ($i = 0; $i <= $#key_words; $i++)  {
		$has_key_words[$i] = 0;
		$has_flags[$i] = 0;
	}

	# flag: FMT_SPLIT_UNIFIES_CASE
	$is_has_split_unifies_case = 0;
	$is_has_FMT_SPLIT_UNIFIES_CASE = 0;

	open($file_handler, "<", @_)
		|| die "failed to open @_\n";
	while ($line = <$file_handler>) {
		for ($i = 0; $i <= $#key_words; $i++) {
			if ($line =~ /$key_words[$i]/) {
				$has_key_words[$i] = 1;
			}
			if ($line =~ /$flags[$i]/) {
				$has_flags[$i] = 1;
			}
		}

		# flag: FMT_SPLIT_UNIFIES_CASE
		if ($line =~ /strlwr/ || $line =~ /strupr/ || $line =~ /enc_strlwr/ || $line =~ /enc_strupper/) {
			$is_has_split_unifies_case = 1;
		}
		if ($line =~ /FMT_SPLIT_UNIFIES_CASE/) {
			$is_has_FMT_SPLIT_UNIFIES_CASE = 1;
		}
	}

	for ($i = 0; $i <= $#key_words; $i++) {
		if ($has_key_words[$i] == 1 && $has_flags[$i] != 1) {
			report_warning(@_, "has key words: $key_words[$i], but no flag: $flags[$i]");
		} elsif ($has_key_words[$i] != 1 && $has_flags[$i] == 1) {
			report_warning(@_, "has flag: $flags[$i], but no key words: $key_words[$i]");
		}
	}

	if ($is_has_split_unifies_case == 1 && $is_has_FMT_SPLIT_UNIFIES_CASE != 1) {
		report_warning(@_, "has key words: strlwr or strupr or enc_strlwr, or enc_strupper, but no flag: FMT_SPLIT_UNIFIES_CASE");
	} elsif ($is_has_split_unifies_case != 1 && $is_has_FMT_SPLIT_UNIFIES_CASE == 1) {
		report_warning(@_, "has flag: FMT_SPLIT_UNIFIES_CASE, but no key words: strlwr or strupr or enc_strlwr, or enc_strupper");
	}

	close($file_handler);
}

sub report_warning {

	$warning_nums = $warning_nums + 1;

	open(LOG, ">> fmt_flags_warning.log") || die;
	print LOG "file    : $_[0]\n";
	print LOG "warning : $_[1]\n\n";
	close(LOG);
}

Powered by blists - more mailing lists

Your e-mail address:

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