Openwall GNU/*/Linux - a small security-enhanced Linux distro for servers
[<prev] [next>] [thread-next>] [day] [month] [year] [list]
Date: Tue, 10 Mar 2015 09:31:16 +0800
From: Kai Zhao <loverszhao@...il.com>
To: john-dev@...ts.openwall.com
Subject: Bug in Siemens-s7 format

Thanks to john-dev's help, I found my first bug in john jumbo.

File: src/siemens-s7-format
Branch: bleeding-jumbo
https://github.com/magnumripper/JohnTheRipper/blob/bleeding-jumbo/src/siemens-s7_fmt_plug.c

_Bug1_

In valid() function from line 89 to 91. The right format of siemens-s7 are
as follows:

"$siemens-s7$1$599fe00cdb61f76cc6e949162f22c95943468acb$002e45951f62602b2f5d15df217f49da2f5379cb"
                        ^
"$siemens-s7$0$387c1fe4ce97e0e71f5a93b4a9557a947cd40d6c$d7789feee651559a09e2f2d92b57306d2835e209"
                        ^

p points to "1" or "0" field.

line 89:  outcome = atoi(p);
line 90:  if (outcome != 1 && outcome != 0)
line91:       goto bail;

I think the author maybe want to check the field to be "1" or "0", but the
code does not meet the need.
Such as formats also pass the check:

"$siemens-s7$AAAAAAA$387c1fe4ce97e0e71f5a93b4a9557a947cd40d6c$d7789feee651559a09e2f2d92b57306d2835e209"
                        ^

"$siemens-s7$###########$387c1fe4ce97e0e71f5a93b4a9557a947cd40d6c$d7789feee651559a09e2f2d92b57306d2835e209"
                        ^
So I think there is a bug for check the field to be "1" or "0"


_Bug2_

According to the valid() function, the hash below can also pass the valid
check.

"$siemens-s7$1$599fe00cdb61f76cc6e949162f22c95943468acb$0"

                              ^
So, there is a bug in get_binary().

line 120:  static void *get_binary(char *ciphertext)
line 121:  {
line 122:     static union {
line 123:        unsigned char c[BINARY_SIZE];
line 124:        ARCH_WORD dummy;
line 125:    } buf;
line 126:    unsigned char *out = buf.c;
line 127:    char *p;
line 128:    int i;
line 129:    p = strrchr(ciphertext, '#39;) + 1;
line 130:    for (i = 0; i < BINARY_SIZE; i++) {
line 131:        out[i] =
line 132:                  (atoi16[ARCH_INDEX(*p)] << 4)  |
line 133:                  atoi16[ARCH_INDEX(p[1])];
line 134:        p += 2;
line 135:    }
line 136:
line 137:    return out;
line 138: }

In line 129, p points to "0", and BINARY_SIZE is 20. But in line 132 and
133 it tries
to visit *p and p[1], also p += 2, thus it will visit the unknown ares!


_Conclusion_

I think the two bugs can be fixed to add check in valid().
Thanks for those who help me. It is inspiring to find the first bug!

sincerely,

Kai

[ CONTENT OF TYPE text/html SKIPPED ]

Powered by blists - more mailing lists

Your e-mail address:

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