[<prev] [next>] [thread-next>] [day] [month] [year] [list]
Message-ID: <CABtNtWGEGMq+KNScfdG-hqpuiifkRH8iPfVsMGZa3i8g=vupsw@mail.gmail.com>
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
Confused about mailing lists and their use?
Read about mailing lists on Wikipedia
and check out these
guidelines on proper formatting of your messages.
