Follow @Openwall on Twitter for new release announcements and other news
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CABtNtWEnaMpjZ5RYEuFfOBFukZj2=7e3j3aJqQPOxKyxqMFp5Q@mail.gmail.com>
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

View attachment "detect_improper_flags.pl" of type "text/x-perl-script" (2295 bytes)

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.