Off by one in action_benchmark

[GoogleCodeExporter]

Compiling cryptsetup with gcc 4.8 and -fsanitize=address and running the test 
suite results in:

=================================================================
==20249== ERROR: AddressSanitizer: unknown-crash on address 0x7fff6552ad90 at 
pc 0x7f016ac2d97f bp 0x7fff6552aaf0 sp 0x7fff6552aaa8
WRITE of size 33 at 0x7fff6552ad90 thread T0
    #0 0x7f016ac2d97e (/usr/lib64/libasan.so.0.0.0+0xb97e)
    #1 0x7f016ac2e022 (/usr/lib64/libasan.so.0.0.0+0xc022)
    #2 0x7f016ac2e12b (/usr/lib64/libasan.so.0.0.0+0xc12b)
    #3 0x403a75 (/home/crrodriguez/scm/cryptsetup/src/.libs/lt-cryptsetup+0x403a75)
    #4 0x40b4c4 (/home/crrodriguez/scm/cryptsetup/src/.libs/lt-cryptsetup+0x40b4c4)
    #5 0x40c1d8 (/home/crrodriguez/scm/cryptsetup/src/.libs/lt-cryptsetup+0x40c1d8)
    #6 0x7f0169dc7a14 (/lib64/libc-2.17.so+0x21a14)
    #7 0x4033a8 (/home/crrodriguez/scm/cryptsetup/src/.libs/lt-cryptsetup+0x4033a8)
Address 0x7fff6552ad90 is located at offset 224 in frame <action_benchmark> of 
T0's stack:
  This frame has 5 object(s):
    [32, 36) 'r'
    [96, 104) 'enc_mbr'
    [160, 168) 'dec_mbr'
    [224, 256) 'cipher'
    [288, 320) 'cipher_mode'
HINT: this may be a false positive if your program uses some custom stack 
unwind mechanism or swapcontext
      (longjmp and C++ exceptions *are* supported)
Shadow bytes around the buggy address:
  0x10006ca9d560: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x10006ca9d570: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x10006ca9d580: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x10006ca9d590: 00 00 00 00 00 00 f1 f1 f1 f1 04 f4 f4 f4 f2 f2
  0x10006ca9d5a0: f2 f2 00 f4 f4 f4 f2 f2 f2 f2 00 f4 f4 f4 f2 f2
=>0x10006ca9d5b0: f2 f2[00]00 00 00 f2 f2 f2 f2 00 00 00 00 f3 f3
  0x10006ca9d5c0: f3 f3 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x10006ca9d5d0: 00 00 00 00 f1 f1 f1 f1 04 f4 f4 f4 f2 f2 f2 f2
  0x10006ca9d5e0: 00 f4 f4 f4 f2 f2 f2 f2 00 00 00 00 00 00 00 00
  0x10006ca9d5f0: 00 00 00 00 00 00 00 00 f3 f3 f3 f3 00 00 00 00
  0x10006ca9d600: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
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
  Heap righ redzone:     fb
  Freed Heap region:     fd
  Stack left redzone:    f1
  Stack mid redzone:     f2
  Stack right redzone:   f3
  Stack partial redzone: f4
  Stack after return:    f5
  Stack use after scope: f8
  Global redzone:        f9
  Global init order:     f6
  Poisoned by user:      f7
  ASan internal:         fe
==20249== ABORTING

Apparently "cipher" and "cipher_mode" are not big enough to hold the needed 
amount of data

--- a/src/cryptsetup.c
+++ b/src/cryptsetup.c
@@ -478,7 +478,7 @@ static int action_benchmark(void)
        static const char *bkdfs[] = {
                "sha1", "sha256", "sha512", "ripemd160", "whirlpool", NULL
        };
-       char cipher[MAX_CIPHER_LEN], cipher_mode[MAX_CIPHER_LEN];
+       char cipher[MAX_CIPHER_LEN + 1], cipher_mode[MAX_CIPHER_LEN + 1];
        double enc_mbr = 0, dec_mbr = 0;
        int key_size = (opt_key_size ?: DEFAULT_PLAIN_KEYBITS);
        int iv_size = 16, skipped = 0;

Original issue reported on code.google.com by judas.iscariote on 14 Jul 2013 at 7:57

[GoogleCodeExporter]

Hm. If this patch really helps, then the real problem is elsewhere 
(crypt_parse_name_and_mode and sscanf) so the patch is IMHO just papering the 
real problem (there is more places).

Thanks for reporting, the whole MAX_CIPHER_LEN needs change (it was based on 
LUKS header limit, but now we support more formats).

Original comment by gmazyl...@gmail.com on 14 Jul 2013 at 9:46

[GoogleCodeExporter]

For now fixed with
http://code.google.com/p/cryptsetup/source/detail?r=cfeaaa02fc50f7cab28c91f78d0f
2da809bc0be1#

Thanks for report!

Original comment by gmazyl...@gmail.com on 23 Jul 2013 at 8:11

• Changed state: Fixed