search

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

test failure because of a global-buffer-overflow #5356

Open asarubbo opened 1 year ago

asarubbo commented 1 year ago

Our Gentoo Tinderbox reported a test failure at bug 914171

By looking at build.log I can see:

==26==ERROR: AddressSanitizer: global-buffer-overflow on address 0x5643cdb3e067 at pc 0x5643cda7f4d9 bp 0x7ffc641ecb90 sp 0x7ffc641ecb88
READ of size 8 at 0x5643cdb3e067 thread T0
    #0 0x5643cda7f4d8 in DES_std_set_key /var/tmp/portage/app-crypt/johntheripper-1.9.0/work/john-1.9.0/src/DES_std.c:663:15
    #1 0x5643cdab5fa0 in set_key /var/tmp/portage/app-crypt/johntheripper-1.9.0/work/john-1.9.0/src/BSDI_fmt.c:266:2
    #2 0x5643cdafac74 in fmt_self_test_body /var/tmp/portage/app-crypt/johntheripper-1.9.0/work/john-1.9.0/src/formats.c:138:3
    #3 0x5643cdafac74 in fmt_self_test /var/tmp/portage/app-crypt/johntheripper-1.9.0/work/john-1.9.0/src/formats.c:227:11
    #4 0x5643cdae58b2 in benchmark_format /var/tmp/portage/app-crypt/johntheripper-1.9.0/work/john-1.9.0/src/bench.c:155:15
    #5 0x5643cdae6bd9 in benchmark_all /var/tmp/portage/app-crypt/johntheripper-1.9.0/work/john-1.9.0/src/bench.c:323:17
    #6 0x5643cdb032e4 in john_run /var/tmp/portage/app-crypt/johntheripper-1.9.0/work/john-1.9.0/src/john.c:565:17
    #7 0x5643cdb032e4 in main /var/tmp/portage/app-crypt/johntheripper-1.9.0/work/john-1.9.0/src/john.c:708:2
    #8 0x7f06d4823c89  (/lib64/libc.so.6+0x23c89)
    #9 0x7f06d4823d44 in __libc_start_main (/lib64/libc.so.6+0x23d44)
    #10 0x5643cd9a4550 in _start (/var/tmp/portage/app-crypt/johntheripper-1.9.0/work/john-1.9.0/run/john+0x37550)

0x5643cdb3e067 is located 57 bytes before global variable '.str' defined in '/var/tmp/portage/app-crypt/johntheripper-1.9.0/work/john-1.9.0/src/MD5_fmt.c:244' (0x5643cdb3e0a0) of size 9
  '.str' is ascii string 'md5crypt'
0x5643cdb3e067 is located 6 bytes after global variable '.str.28' defined in '/var/tmp/portage/app-crypt/johntheripper-1.9.0/work/john-1.9.0/src/BSDI_fmt.c:44' (0x5643cdb3e060) of size 1
  '.str.28' is ascii string ''
SUMMARY: AddressSanitizer: global-buffer-overflow /var/tmp/portage/app-crypt/johntheripper-1.9.0/work/john-1.9.0/src/DES_std.c:663:15 in DES_std_set_key
Shadow bytes around the buggy address:
  0x5643cdb3dd80: f9 f9 f9 f9 00 00 01 f9 f9 f9 f9 f9 00 00 05 f9
  0x5643cdb3de00: f9 f9 f9 f9 00 00 02 f9 f9 f9 f9 f9 00 00 02 f9
  0x5643cdb3de80: f9 f9 f9 f9 00 00 05 f9 f9 f9 f9 f9 00 02 f9 f9
  0x5643cdb3df00: 00 00 05 f9 f9 f9 f9 f9 00 02 f9 f9 00 00 05 f9
  0x5643cdb3df80: f9 f9 f9 f9 00 07 f9 f9 00 00 05 f9 f9 f9 f9 f9
=>0x5643cdb3e000: 00 01 f9 f9 00 00 05 f9 f9 f9 f9 f9[01]f9 f9 f9
  0x5643cdb3e080: 00 00 00 00 00 01 f9 f9 01 f9 f9 f9 00 05 f9 f9
  0x5643cdb3e100: 00 00 00 00 03 f9 f9 f9 f9 f9 f9 f9 00 00 f9 f9
  0x5643cdb3e180: 00 00 00 00 03 f9 f9 f9 f9 f9 f9 f9 06 f9 f9 f9
  0x5643cdb3e200: 00 00 00 00 06 f9 f9 f9 f9 f9 f9 f9 05 f9 f9 f9
  0x5643cdb3e280: 00 00 00 00 03 f9 f9 f9 f9 f9 f9 f9 00 01 f9 f9
Shadow byte legend (one shadow byte represents 8 application bytes):
  Addressable:           00
  Partially addressable: 01 02 03 04 05 06 07 
  Heap left redzone:       fa
  Freed heap region:       fd
  Stack left redzone:      f1
  Stack mid redzone:       f2
  Stack right redzone:     f3
  Stack after return:      f5
  Stack use after scope:   f8
  Global redzone:          f9
  Global init order:       f6
  Poisoned by user:        f7
  Container overflow:      fc
  Array cookie:            ac
  Intra object redzone:    bb
  ASan internal:           fe
  Left alloca redzone:     ca
  Right alloca redzone:    cb
==26==ABORTING

I didn't look deeply into this issue so I don't know if the bug is in the unittest itself or in the involed libraries/daemons, if so please check for any security implications. If I can do further, please let me know.

claudioandre-br commented 1 year ago

Thanks for reporting.

I'm afraid I cannot reproduce this using latest bleeding Jumbo, but I can if I use the latest stable (john proper) version:

commit d747b444654ef9454fd193b449aeb5f4c1091b28 (HEAD -> core, origin/core)
Merge: d64d6434e a363d6d50
Author: Solar <solar@openwall.com>
Date:   Sun May 19 15:14:35 2019 +0000

    cvsimport

    * refs/remotes/cvs/master:
      Copyright years update
$ run/john
John the Ripper password cracker, version 1.9.0
Copyright (c) 1996-2019 by Solar Designer
Homepage: https://www.openwall.com/john/

Usage: john [OPTIONS] [PASSWORD-FILES]
--single                   "single crack" mode
--wordlist=FILE --stdin    wordlist mode, read words from FILE or stdin
--rules                    enable word mangling rules for wordlist mode
--incremental[=MODE]       "incremental" mode [using section MODE]
--external=MODE            external mode or word filter
--stdout[=LENGTH]          just output candidate passwords [cut at LENGTH]
--restore[=NAME]           restore an interrupted session [called NAME]
--session=NAME             give a new session the NAME
--status[=NAME]            print status of a session [called NAME]
--make-charset=FILE        make a charset, FILE will be overwritten
--show                     show cracked passwords
--test[=TIME]              run tests and benchmarks for TIME seconds each
--users=[-]LOGIN|UID[,..]  [do not] load this (these) user(s) only
--groups=[-]GID[,..]       load users [not] of this (these) group(s) only
--shells=[-]SHELL[,..]     load users with[out] this (these) shell(s) only
--salts=[-]N               load salts with[out] at least N passwords only
--save-memory=LEVEL        enable memory saving, at LEVEL 1..3
--node=MIN[-MAX]/TOTAL     this node's number range out of TOTAL count
--fork=N                   fork N processes
--format=NAME              force hash type NAME: descrypt/bsdicrypt/md5crypt/
                           bcrypt/LM/AFS/tripcode/dummy/crypt
$ make -j8 -C src/ check
make: Entering directory '/workspaces/bin/bleeding/src'
../run/john --make_check
Will run 8 OpenMP threads
Warning: doing quick benchmarking - the performance numbers will be inaccurate
Benchmarking: descrypt, traditional crypt(3) [DES 256/256 AVX2]... DONE
Many salts:     6553K c/s real, 819200 c/s virtual
Only one salt:  6553K c/s real, 1092K c/s virtual

Benchmarking: bsdicrypt, BSDI crypt(3) ("_J9..", 725 iterations) [DES 256/256 AVX2]... =================================================================
==8605==ERROR: AddressSanitizer: global-buffer-overflow on address 0x56467942a2e1 at pc 0x5646793a7322 bp 0x7ffc36d34e30 sp 0x7ffc36d34e20
READ of size 1 at 0x56467942a2e1 thread T0
    #0 0x5646793a7321 in DES_std_set_key /workspaces/bin/bleeding/src/DES_std.c:664
    #1 0x5646793cfc90 in set_key /workspaces/bin/bleeding/src/BSDI_fmt.c:266
    #2 0x5646794086ab in fmt_self_test_body /workspaces/bin/bleeding/src/formats.c:138
    #3 0x5646794086ab in fmt_self_test /workspaces/bin/bleeding/src/formats.c:227
    #4 0x5646793f71a8 in benchmark_format /workspaces/bin/bleeding/src/bench.c:155
    #5 0x5646793f7ff4 in benchmark_all /workspaces/bin/bleeding/src/bench.c:323
    #6 0x5646793a2ef7 in john_run /workspaces/bin/bleeding/src/john.c:565
    #7 0x5646793a2ef7 in main /workspaces/bin/bleeding/src/john.c:708
    #8 0x7f86c437ad8f  (/lib/x86_64-linux-gnu/libc.so.6+0x29d8f)
    #9 0x7f86c437ae3f in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x29e3f)
    #10 0x5646793a4464 in _start (/workspaces/bin/bleeding/run/john+0x21464)

0x56467942a2e1 is located 0 bytes to the right of global variable '*.LC30' defined in 'BSDI_fmt.c' (0x56467942a2e0) of size 1
  '*.LC30' is ascii string ''
SUMMARY: AddressSanitizer: global-buffer-overflow /workspaces/bin/bleeding/src/DES_std.c:664 in DES_std_set_key
Shadow bytes around the buggy address:
  0x0ac94f27d400: f9 f9 f9 f9 00 00 02 f9 f9 f9 f9 f9 00 00 02 f9
  0x0ac94f27d410: f9 f9 f9 f9 00 00 05 f9 f9 f9 f9 f9 00 02 f9 f9
  0x0ac94f27d420: f9 f9 f9 f9 00 00 05 f9 f9 f9 f9 f9 00 02 f9 f9
  0x0ac94f27d430: f9 f9 f9 f9 00 00 05 f9 f9 f9 f9 f9 00 07 f9 f9
  0x0ac94f27d440: f9 f9 f9 f9 00 00 05 f9 f9 f9 f9 f9 00 01 f9 f9
=>0x0ac94f27d450: f9 f9 f9 f9 00 00 05 f9 f9 f9 f9 f9[01]f9 f9 f9
  0x0ac94f27d460: f9 f9 f9 f9 00 00 00 00 00 00 00 00 00 00 00 00
  0x0ac94f27d470: 00 00 00 00 00 00 00 00 04 f9 f9 f9 f9 f9 f9 f9
  0x0ac94f27d480: 07 f9 f9 f9 f9 f9 f9 f9 00 01 f9 f9 f9 f9 f9 f9
  0x0ac94f27d490: 01 f9 f9 f9 f9 f9 f9 f9 00 05 f9 f9 f9 f9 f9 f9
  0x0ac94f27d4a0: 00 00 00 00 03 f9 f9 f9 f9 f9 f9 f9 00 00 f9 f9
Shadow byte legend (one shadow byte represents 8 application bytes):
  Addressable:           00
  Partially addressable: 01 02 03 04 05 06 07 
  Heap left redzone:       fa
  Freed heap region:       fd
  Stack left redzone:      f1
  Stack mid redzone:       f2
  Stack right redzone:     f3
  Stack after return:      f5
  Stack use after scope:   f8
  Global redzone:          f9
  Global init order:       f6
  Poisoned by user:        f7
  Container overflow:      fc
  Array cookie:            ac
  Intra object redzone:    bb
  ASan internal:           fe
  Left alloca redzone:     ca
  Right alloca redzone:    cb
  Shadow gap:              cc
==8605==ABORTING
make: *** [Makefile:978: check] Error 1
make: Leaving directory '/workspaces/bin/bleeding/src'

I tried to use the same build flags:

make -j8 -C src/ 'CFLAGS=-c -Wall -I/usr/include -O2 -mtune=generic -march=x86-64 -fno-stack-protector -fcf-protection=none -U_FORTIFY_SOURCE -Wno-implicit-int -Wno-implicit-function-declaration -Wno-int-conversion -Wno-declaration-after-statement -Wno-incompatible-function-pointer-types -Wno-deprecated-non-prototypes -Wno-strict-prototypes -Wno-return-type -g3 -ggdb3 -fno-omit-frame-pointer -fsanitize=address -fPIC -fPIE -fopenmp' 'LDFLAGS=-Wl,-O1 -Wl,--as-needed -Wl,--hash-style=gnu -Wl,-z,norelro -fsanitize=address -fopenmp' OPT_NORMAL= OMPFLAGS=-fopenmp linux-x86-64-avx2

I also can see (other) bad behavior:

$ make -j8 -C src/ check
make: Entering directory '/workspaces/bin/bleeding/src'
../run/john --make_check
Will run 8 OpenMP threads
Warning: doing quick benchmarking - the performance numbers will be inaccurate
Benchmarking: descrypt, traditional crypt(3) [DES 256/256 AVX2]... DONE
Many salts:     13107K c/s real, 1872K c/s virtual
Only one salt:  9830K c/s real, 1966K c/s virtual

Benchmarking: bsdicrypt, BSDI crypt(3) ("_J9..", 725 iterations) [DES 256/256 AVX2]... DONE
Many salts:     409600 c/s real, 81920 c/s virtual
Only one salt:  819200 c/s real, 81920 c/s virtual

Benchmarking: md5crypt [MD5 32/64 X2]... =================================================================
==2489==ERROR: AddressSanitizer: negative-size-param: (size=-1094795586)
    #0 0x7f60286843ff in __interceptor_memcpy ../../../../src/libsanitizer/sanitizer_common/sanitizer_common_interceptors.inc:827
    #1 0x55b1a48031c0 in MD5_std_crypt._omp_fn.0 (/workspaces/bin/bleeding/run/john+0x131c0)
    #2 0x7f6028614a15 in GOMP_parallel (/lib/x86_64-linux-gnu/libgomp.so.1+0x14a15)
    #3 0x55b1a480822c in MD5_std_crypt (/workspaces/bin/bleeding/run/john+0x1822c)
    #4 0x55b1a4802f40 in crypt_all (/workspaces/bin/bleeding/run/john+0x12f40)
    #5 0x55b1a4817981 in fmt_self_test (/workspaces/bin/bleeding/run/john+0x27981)
    #6 0x55b1a481098d in benchmark_format (/workspaces/bin/bleeding/run/john+0x2098d)
    #7 0x55b1a481110c in benchmark_all (/workspaces/bin/bleeding/run/john+0x2110c)
    #8 0x55b1a47f6cdf in main (/workspaces/bin/bleeding/run/john+0x6cdf)
    #9 0x7f6028401d8f  (/lib/x86_64-linux-gnu/libc.so.6+0x29d8f)
    #10 0x7f6028401e3f in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x29e3f)
    #11 0x55b1a47f7634 in _start (/workspaces/bin/bleeding/run/john+0x7634)

0x7f6020adb7d0 is located 4048 bytes inside of 2363391-byte region [0x7f6020ada800,0x7f6020d1b7ff)
allocated by thread T0 here:
    #0 0x7f60286fe867 in __interceptor_malloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:145
    #1 0x55b1a481cef1 in mem_alloc_tiny (/workspaces/bin/bleeding/run/john+0x2cef1)

SUMMARY: AddressSanitizer: negative-size-param ../../../../src/libsanitizer/sanitizer_common/sanitizer_common_interceptors.inc:827 in __interceptor_memcpy
==2489==ABORTING
make: *** [Makefile:978: check] Error 1
make: Leaving directory '/workspaces/bin/bleeding/src'

No CFLAGS was set to see the above error. make -j8 -C src/ 'CPPFLAGS=-c -Wall -I/usr/include -I/usr/lib/llvm-11/lib/clang/11.1.0/include -O2 -mtune=generic -march=x86-64 -fno-stack-protector -fcf-protection=none -U_FORTIFY_SOURCE -Wno-implicit-int -Wno-implicit-function-declaration -Wno-int-conversion -Wno-declaration-after-statement -Wno-incompatible-function-pointer-types -Wno-deprecated-non-prototypes -Wno-strict-prototypes -Wno-return-type -g3 -ggdb3 -fno-omit-frame-pointer -fsanitize=address -fPIC -fPIE -fopenmp' 'LDFLAGS=-Wl,-O1 -Wl,--as-needed -Wl,--hash-style=gnu -Wl,-z,norelro -fsanitize=address -fopenmp' OPT_NORMAL= OMPFLAGS=-fopenmp linux-x86-64-avx2

Final note:

claudioandre-br commented 1 year ago

IMO, we should tag something as Jumbo 2 (this being a maintenance release with known bugs).

We don't need to use the 'old school' release process for evolving (maintenance) releases. Maybe, think about it as minor (changes) releases.

Ok, in this case we will create a major and breaking release.

solardiz commented 1 year ago

IIRC, DES_std.c set_key has some intentional over-reads, which are worked around in jumbo:

commit 74706da0eedb724651dff8e777cb777b24b37222
Author: JimF <jfoug@openwall.net>
Date:   Wed Dec 17 16:06:01 2014 -0600

    DES/bench: Fix issue where we past and later dereference constant strings past their end (ASAN find)

commit 351dc170509ee63ae8aee30ce506a3ddcd1a61e8
Author: Dhiru Kholia <dhiru@openwall.com>
Date:   Wed Dec 17 15:37:57 2014 +0100

    Exclude some known-good methods from ASan instrumentation

commit 03dceefd6fe0d18cc3cd848a6b5fc83ef9ac811d
Author: JimF <jfoug@openwall.net>
Date:   Fri Dec 12 13:34:13 2014 -0600

    DES: fixes a LONG standing compiler warning

See also: #276.

We might want to come up with proper fixes for them instead of these workarounds.

I don't know about other issues @claudioandre-br mentions here. These could need to be looked into separately if we care, although at this point there probably isn't going to be another john-core release, so if an issue is not seen in jumbo we're good.

I don't see how the suggestion to tag anything as Jumbo 2 is relevant to any of these issues. I think it is not.