Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Incorrect shadow values for global string constants on OSX #274

Closed
ramosian-glider opened this issue Aug 31, 2015 · 7 comments
Closed

Incorrect shadow values for global string constants on OSX #274

ramosian-glider opened this issue Aug 31, 2015 · 7 comments

Comments

@ramosian-glider
Copy link
Member

Originally reported on Google Code with ID 274

See http://crbug.com/352073

=================================================================
==38424==ERROR: AddressSanitizer: global-buffer-overflow on address 0x000f8362 at pc
0x14b16d bp 0xbff7f5f8 sp 0xbff7f5e8
READ of size 1 at 0x000f8362 thread T0
    #0 0x14b16c in wrap_memmove (/Volumes/data/b/build/slave/mac_asan/build/src/third_party/llvm-build/Release+Asserts/lib/clang/3.5/lib/darwin/libclang_rt.asan_osx_dynamic.dylib+0x1716c)
    #1 0x965fe351 in __CFStringAppendBytes (/System/Library/Frameworks/CoreFoundation.framework/Versions/A/CoreFoundation+0x8351)
    #2 0x965fd99e in __CFStringAppendFormatCore (/System/Library/Frameworks/CoreFoundation.framework/Versions/A/CoreFoundation+0x799e)
    #3 0x9664a19b in _CFStringCreateWithFormatAndArgumentsAux (/System/Library/Frameworks/CoreFoundation.framework/Versions/A/CoreFoundation+0x5419b)
    #4 0x9575beed in -[NSPlaceholderString initWithFormat:locale:arguments:] (/System/Library/Frameworks/Foundation.framework/Versions/C/Foundation+0x5beed)
    #5 0x9575d04b in +[NSString stringWithFormat:] (/System/Library/Frameworks/Foundation.framework/Versions/C/Foundation+0x5d04b)
    #6 0x80ab8 in main (/Volumes/data/b/build/slave/mac_asan/build/src/chrome/../out/Release/infoplist_strings_tool+0x2ab8)
    #7 0x80254 in start (/Volumes/data/b/build/slave/mac_asan/build/src/chrome/../out/Release/infoplist_strings_tool+0x2254)

0x000f8362 is located 2 bytes inside of global variable '.str119' from '../../chrome/tools/mac_helpers/infoplist_strings_util.mm'
(0xf8360) of size 12
  '.str119' is ascii string '%d.%d.%d.%d'
0x000f8362 is located 27 bytes to the right of global variable '.str117' from '../../chrome/tools/mac_helpers/infoplist_strings_util.mm'
(0xf8340) of size 7
  '.str117' is ascii string 'PATCH='
SUMMARY: AddressSanitizer: global-buffer-overflow ??:0 wrap_memmove
Shadow bytes around the buggy address:
  0x2001f010: 03 f9 f9 f9 f9 f9 f9 f9 00 00 00 00 02 f9 f9 f9
  0x2001f020: 03 f9 f9 f9 02 f9 f9 f9 03 f9 f9 f9 02 f9 f9 f9
  0x2001f030: 03 f9 f9 f9 02 f9 f9 f9 03 f9 f9 f9 f9 f9 f9 f9
  0x2001f040: 00 00 00 00 00 05 f9 f9 f9 f9 f9 f9 00 00 00 00
  0x2001f050: 00 00 00 00 00 00 00 00 07 f9 f9 f9 f9 f9 f9 f9
=>0x2001f060: 07 f9 f9 f9 07 f9 f9 f9 07 f9 f9 f9[f9]04 f9 f9
  0x2001f070: f9 f9 f9 f9 00 00 00 00 00 00 00 00 00 00 00 00
  0x2001f080: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x2001f090: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x2001f0a0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x2001f0b0: 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 right 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
  Contiguous container OOB:fc
  ASan internal:           fe
==38424==ABORTING
ninja: build stopped: subcommand failed.

Reported by ramosian.glider on 2014-03-13 09:58:37

@ramosian-glider
Copy link
Member Author

From a local run with ASAN_OPTIONS=report_globals=2:

...
==55507==Added Global: beg=0x00156340 size=7/64 name=.str117 module=../../chrome/tools/mac_helpers/infoplist_strings_util.mm
dyn_init=0
==55507==Added Global: beg=0x00156360 size=12/64 name=.str119 module=../../chrome/tools/mac_helpers/infoplist_strings_util.mm
dyn_init=0
...
==55507==Search Global: beg=0x00156360 size=12/64 name=.str119 module=../../chrome/tools/mac_helpers/infoplist_strings_util.mm
dyn_init=0
0x00156362 is located 2 bytes inside of global variable '.str119' from '../../chrome/tools/mac_helpers/infoplist_strings_util.mm'
(0x156360) of size 12
  '.str119' is ascii string '%d.%d.%d.%d'
==55507==Search Global: beg=0x00156340 size=7/64 name=.str117 module=../../chrome/tools/mac_helpers/infoplist_strings_util.mm
dyn_init=0
0x00156362 is located 27 bytes to the right of global variable '.str117' from '../../chrome/tools/mac_helpers/infoplist_strings_util.mm'
(0x156340) of size 7
  '.str117' is ascii string 'PATCH='
...


According to -emit-llvm the size of .str117 is 64 bytes:

@.str117 = internal unnamed_addr constant { [7 x i8], [57 x i8] } { [7 x i8] c"PATCH=\00",
[57 x i8] zeroinitializer }, section "__TEXT,__cstring,cstring_literals", align 32

, however the next string starts at .str117+32.

Hexdump also shows that the variables are too close to each other:

00079340  50 41 54 43 48 3d 00 00  00 00 00 00 00 00 00 00  |PATCH=..........|
00079350  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  |................|
00079360  25 64 2e 25 64 2e 25 64  2e 25 64 00 00 00 00 00  |%d.%d.%d.%d.....|

Reported by ramosian.glider on 2014-03-13 10:03:41

@ramosian-glider
Copy link
Member Author

The assembly looks sane:

        .section        __TEXT,__cstring,cstring_literals
        .align  5                       ## @.str117
_.str117:
        .asciz  "PATCH="
        .space  57

        .section        __DATA,__cfstring
        .align  3                       ## @_unnamed_cfstring_118
L__unnamed_cfstring_118:
        .long   ___CFConstantStringClassReference
        .long   1992                    ## 0x7c8
        .long   _.str117
        .long   6                       ## 0x6

        .section        __TEXT,__cstring,cstring_literals
        .align  5                       ## @.str119
_.str119:
        .asciz  "%d.%d.%d.%d"
        .space  52


Perhaps the linker removes the padding, but keeps the alignment?

Reported by ramosian.glider on 2014-03-13 10:14:11

@ramosian-glider
Copy link
Member Author

This is a regression caused by the recent changes to string handling.

According to https://code.google.com/p/address-sanitizer/issues/detail?id=32 the strings
in __TEXT,__cstring,cstring_literals were previously marked as linker_private and thus
not instrumented. Now they've become internal, so ASan instruments them.

However strings in the cstring_literals section are mergeable:

"""
A cstring_literals section contains null-terminated literal C language character strings.
The link editor places only one copy of each literal into the output file’s section
and relocates references to different copies of the same literal to the one copyin
the output file. There can be no relocation entries for a section of this type, and
all references to literals in this section must be inside the address range for the
specific literal being referenced. The last byte in a section of this type must be
a null byte, and the strings can’t contain null bytes in their bodies. An example of
a cstring_literals section is one for the literal strings that appear in the body of
an ANSI C function where the compiler chooses to make such strings read only.
"""

(from the Mac OS Assembler Guide)

, thus we should not instrument them.

Reported by ramosian.glider on 2014-03-13 12:06:34

@ramosian-glider
Copy link
Member Author

Minimal repro below:

#import <Foundation/Foundation.h>

#include <stdio.h>

int main() {
  NSString* version_file = @"MAJOR=35\n";
  int major = 0, minor = 0, build = 0, patch = 0;
  NSScanner* scanner = [NSScanner scannerWithString:version_file];
  NSString *res = nil;
  if ([scanner scanString:@"MAJOR=" intoString:nil] &&
      [scanner scanInt:&major]) {
    res = [NSString stringWithFormat:@"%d.%d.%d.%d",
           major, minor, build, patch];
  }
  printf("%s\n", [res UTF8String]);
  return 0;
}

Reported by ramosian.glider on 2014-03-13 12:06:55

@ramosian-glider
Copy link
Member Author

Reported by glider@chromium.org on 2014-03-13 12:51:33

  • Blocking: #352073

@ramosian-glider
Copy link
Member Author

Fixed in Clang r203916.

Reported by ramosian.glider on 2014-03-14 13:51:56

  • Status changed: Fixed

@ramosian-glider
Copy link
Member Author

Adding Project:AddressSanitizer as part of GitHub migration.

Reported by ramosian.glider on 2015-07-30 09:14:07

  • Labels added: ProjectAddressSanitizer

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant