openwall / john

John the Ripper jumbo - advanced offline password cracker, which supports hundreds of hash and cipher types, and runs on many operating systems, CPUs, GPUs, and even some FPGAs
https://www.openwall.com/john/
Other
10.29k stars 2.1k forks source link

mssql12 segfault with --fork #1076

Closed frank-dittrich closed 9 years ago

frank-dittrich commented 9 years ago
(master)test $ ./jtrts.pl -v -stoponerror -type mssql12 -passthru="--fork=2"
-------------------------------------------------------------------------------
- JtR-TestSuite (jtrts). Version 1.13, Dec 21, 2014.  By, Jim Fougeron & others
- Testing:  John the Ripper password cracker, version 1.8.0.2-jumbo-1-bleeding_omp [linux-gnu 64-bit AVX-autoconf]
--------------------------------------------------------------------------------

John Jumbo build detected.
--pot=NAME option is valid
--encoding=NAME option is valid
all.chr (../run/all.chr) not found
alnum.chr found, inc_alnum_ee8763c850dee8e4b88ef547a8ed39b8 added as a capability
Types to filter on:
mssql12
Capabilities in this build of john:
jumbo core inc local_pot_valid encode_valid utf8 cp1252 cp1251 koi8r cp437 cp737 cp850 cp858 cp866 iso8859-1 iso8859-15 inc_alnum_ee8763c850dee8e4b88ef547a8ed39b8
sh: line 1: 25175 Segmentation fault      (core dumped) ../run/john -ses=./tst --fork=2 -pot=./tst.pot MsSql12_tst.in --wordlist=pw.dic 2>&1 > /dev/null

Use of uninitialized value $crack_xx[3] in concatenation (.) or string at ./jtrts.pl line 860.
Use of uninitialized value in concatenation (.) or string at ./jtrts.pl line 860.
form=MSSql12                      guesses:    0    [pass, but return code 35584]
Exiting on error. The .pot file ./tst.pot contains the found data
The command used to run this test was:

../run/john -ses=./tst  --fork=2 -pot=./tst.pot MsSql12_tst.in --wordlist=pw.dic
magnumripper commented 9 years ago

I believe --fork has absolutely nothing to do with this problem. You seem to focus a whole lot on using fork with TS but it's just confusing and misleading. Formats are 100% unaffected by the fork option, only core and modes are affected by it.

magnumripper commented 9 years ago

I can reproduce it with -fork or -node but not without it. This is more likely a bug in wordlist mode. (So yes I was wrong, and also right o.O)

frank-dittrich commented 9 years ago

I started with --fork=3, got the segfault, re-tested without fork, and for me the error disappeared:

(master)test $ ./jtrts.pl -type mssql12 -v -stoponerror
-------------------------------------------------------------------------------
- JtR-TestSuite (jtrts). Version 1.13, Dec 21, 2014.  By, Jim Fougeron & others
- Testing:  John the Ripper password cracker, version 1.8.0.2-jumbo-1-bleeding_omp [linux-gnu 64-bit AVX-autoconf]
--------------------------------------------------------------------------------

John Jumbo build detected.
--pot=NAME option is valid
--encoding=NAME option is valid
all.chr (../run/all.chr) not found
alnum.chr found, inc_alnum_ee8763c850dee8e4b88ef547a8ed39b8 added as a capability
Types to filter on:
mssql12
Capabilities in this build of john:
jumbo core inc local_pot_valid encode_valid utf8 cp1252 cp1251 koi8r cp437 cp737 cp850 cp858 cp866 iso8859-1 iso8859-15 inc_alnum_ee8763c850dee8e4b88ef547a8ed39b8

form=MSSql12                      guesses: 1500 0:00:00:00 DONE  [PASSED]
.pot CHK:MSSql12                  guesses: 1500 0:00:00:00 DONE  [PASSED] (1500 val-pwd)

All tests passed without error.  Performed 1 tests.  Time used was 2 seconds

Then, I tested --fork=2, and reported the error.

magnumripper commented 9 years ago

This hides the bug:

diff --git a/src/mssql12_fmt_plug.c b/src/mssql12_fmt_plug.c
index 1f8a3e0..cb8969b 100644
--- a/src/mssql12_fmt_plug.c
+++ b/src/mssql12_fmt_plug.c
@@ -252,10 +252,13 @@ static char *get_key(int index)
        ARCH_WORD_64 *keybuffer = saved_key[index];
        UTF16 *w16 = (UTF16*)keybuffer;
        static UTF16 out[PLAINTEXT_LENGTH + 1];
-       unsigned int i, len;
+       int i, len;

        len = ((keybuffer[15] >> 3) - SALT_SIZE) >> 1;

+       if (len < 0)
+               len = 0;
+
        for(i = 0; i < len; i++)
                out[i] = w16[i];

However, I think this is a bug in core that needs to be fixed instead. Either we call get_key() on an index where we never set the key, or we called clear_keys() and after that, called get_key() from status.c

magnumripper commented 9 years ago
(gdb) bt
#0  0x000000010015a3fc in get_key (index=<value temporarily unavailable, due to optimizations>) at mssql12_fmt_plug.c:260
#1  0x00000001002626d3 in status_print () at status.c:325
magnumripper commented 9 years ago
Swor?fish?_?     (u1228-mssql12)
get_key(255)
summer__5        (u469-mssql12)
get_key(754)
summer__17       (u1452-mssql12)
get_key(44)
Dec 18, 1992     (u70-mssql12)
clear_keys()
status_print_cracking()
get_key(2934)
Bus error: 10

Sure enough, that is what happens. status.c calls get_key() after clear_keys() was called. This might be a somewhat tricky problem.

magnumripper commented 9 years ago

I wonder if this is actually supposed to be supported. Maybe we should ask Solar. If this is supposed to be OK, the patch I posted is the definite fix.

magnumripper commented 9 years ago

http://www.openwall.com/lists/john-dev/2015/03/08/1

magnumripper commented 9 years ago

Still cores

magnumripper commented 9 years ago

WhyTF do I not get a proper BT?

$ rm -f tst.pot && OMP_NUM_THREADS=1 gdb --args ../run/john -ses=./tst -node=1/2 -pot=./tst.pot MsSql12_tst.in --wordlist=pw.dic
(...)
(gdb) r
Starting program: /Users/magnum/src/john/run/john -ses=./tst -node=1/2 -pot=./tst.pot MsSql12_tst.in --wordlist=pw.dic
Loaded 1500 password hashes with 1500 different salts (mssql12, MS SQL 2012/2014 [SHA512 128/128 SSE4.1 2x])
Warning: OpenMP is disabled; a non-OpenMP build may be faster
Node number 1 of 2
Press 'q' or Ctrl-C to abort, almost any other key for status
swordfish__10    (u912-mssql12)
swordfish__6     (u568-mssql12)
(...)
summer__5        (u469-mssql12)
summer__17       (u1452-mssql12)
Dec 18, 1992     (u70-mssql12)

Program received signal EXC_BAD_ACCESS, Could not access memory.
Reason: KERN_INVALID_ADDRESS at address: 0x0000000102cf8000
0x000000010015b2f6 in get_key (index=<value temporarily unavailable, due to optimizations>) at mssql12_fmt_plug.c:260
260         out[i] = w16[i];
(gdb) bt
#0  0x000000010015b2f6 in get_key (index=<value temporarily unavailable, due to optimizations>) at mssql12_fmt_plug.c:260
#1  0x00000001002635d3 in status_print () at status.c:325
#2  0x00007fff5fbfebd0 in ?? ()
#3  0x00007fff5fc2add1 in __dyld_write ()
#4  0x00007fff5fc3fb80 in __dyld__ZL18initialPoolContent ()
Previous frame inner to this frame (gdb could not unwind past this frame)
(gdb) 

I would love to know frame 2.

magnumripper commented 9 years ago

frame 2 was john.c line 1420...