[AudioToolbox] MP4AudioFile::Close crashes writing large files

Originator:jeremyw.sherman
Number:rdar://7230346 Date Originated:16-Sep-2009 08:25 PM
Status:Duplicate/7100093 Resolved:
Product:Mac OS X Product Version:10.6/10A432
Classification:Crash/Hang/Data Loss Reproducible:Always
 
Summary:
Calling ExtAudioFileDispose() on a sufficiently large MP4 audio file causes a crash. All crash reports have the same final several stack frames within the AudioToolbox.

The salient details are:
  * ExtAudioFileDispose() calls AudioFileClose()
  * AudioFileClose() calls MP4AudioFile::Close()
  * As part of closing the mp4 file, MP4AudioFile::FillGaplessString() is called
  * MP4AudioFile::FillGaplessString() calls UInt32ToHexString with a sufficiently large value (value >= 0x10000000; see NOTES below)
  * UInt32ToHexString() calls sprintf() with a too-small stack-allocated buffer
  * At the end of UInt32ToHexString(), __stack_chk_guard is found to have been altered, and __stack_chk_fail() is called.
  * __stack_chk_fail() logs a message and calls __abort(), which terminates the process.

See NOTES for the corresponding excerpt from a crash report and the attached crash reports for all details.

This bug affects the AudioToolbox framework distributed with both Mac OS X 10.6 and 10.6.1.


Steps to Reproduce:
  1. Download ProfCast (see http://www.profcast.com/downloads/)
  2. Install ProfCast.
  3. Launch ProfCast.
  4. Select Create a New Recording from the initial window (or hit Cmd-N, or choose File > New Recording).
  5. Click the Start Recording button in the upper left portion of the window that appears.
  6. Record for a long time – 2 hours works, 45 minutes does not.
  7. Click "Pause Recording".


Expected Results:
Recording pauses and the button changes to say "Resume Recording". Clicking the button resumes recording.


Actual Results:
The application crashes while attempting to write the audio file. The audio file is incomplete and unusable, leading to total data loss.


Regression:
The code involved has worked fine for years for users running Mac OS X 10.4 and 10.5. MP4AudioFile::FillGaplessString and UInt32ToHexString do not exist in Mac OS X 10.5's AudioToolbox framework. This is a new bug introduced with Mac OS X 10.6 and still present in Mac OS X 10.6.1.


Notes:
Since upgrading to Mac OS X 10.6, some users of ProfCast have reported crashes leading to loss of the entire recording when they pause recording. Their crash reports are all due to this issue. This is a serious problem for institutional users of ProfCast who rely on the application to record lectures or other events.

One workaround might be breaking up a long recording session into several smaller sessions, but this is less than ideal.

The crash report stack frames within the AudioToolbox are these:
Thread 0 Crashed:  Dispatch queue: com.apple.main-thread
0   libSystem.B.dylib             	0x965e0972 __kill + 10
1   libSystem.B.dylib             	0x965e0964 kill$UNIX2003 + 32
2   libSystem.B.dylib             	0x96673ba5 raise + 26
3   libSystem.B.dylib             	0x96689bf1 __abort + 124
4   libSystem.B.dylib             	0x9666c934 release_file_streams_for_task + 0
5   ....audio.toolbox.AudioToolbox	0x98267497 UInt32ToHexString(char*, unsigned long, unsigned long) + 183
6   ....audio.toolbox.AudioToolbox	0x98267556 MP4AudioFile::FillGaplessString(char*) + 110
7   ....audio.toolbox.AudioToolbox	0x9826a3ed MP4AudioFile::Close() + 627
8   ....audio.toolbox.AudioToolbox	0x981c7a25 AudioFileClose + 77
9   ....audio.toolbox.AudioToolbox	0x9822aef6 ExtAudioFile::Close() + 206
10  ....audio.toolbox.AudioToolbox	0x9822b03e ExtAudioFile::~ExtAudioFile() + 40
11  ....audio.toolbox.AudioToolbox	0x9822089c XExtAudioFile::~XExtAudioFile() + 92
12  ....audio.toolbox.AudioToolbox	0x98220554 ExtAudioFileDispose + 37

Note that release_file_streams_for_task + 0 is the instruction following the last instruction of __stack_chk_fail, which jumps directly to __abort and never returns.

The stack check failure in UInt32ToHexString() is easy to trigger directly. It occurs whenever the most-significant half-byte of the UInt32 value supplied for conversion to a hex string is non-null, that is, for all UInt32 values >= 0x10000000 (1 + 7 zeros). This appears to be because the function supplies an 8-byte stack buffer to sprintf together with a "%X" format string. The crashing values require 9 characters: 8 characters to represent the value itself plus 1 string-final NUL byte. Using a 9-byte buffer in UInt32ToHexString() would fix the problem. Using snprintf() instead of sprintf() would mask the problem by failing to write the least significant hex digit.

Attachments:
  * Crash Reports.zip: 4 different crash reports encountered by users of ProfCast, 3 under 10.6.0 and 1 under 10.6.1
  * smash_test.zip: source code demonstrating UInt32ToHexString's stack-smashing and a crash report produced under 10.6.1 by compiling and running a messier ancestor of the supplied source code

Calling UInt32ToHexString(buffer, buffer_length, 0x10000000) is enough to trigger the crash, but getting access to UInt32ToHexString, a private symbol, requires jumping through some hoops.

Comments

Duplicates rdar://problem/7226263, "Crash in ExtAudioFileDispose".

By jeremyw.sherman at 2009-09-17 01:38:25.588753 (reply...)

smash.mm

/*
 * @file smash.mm
 * @author Jeremy W. Sherman
 * @date 2009-09-11
 *
 * Compile using:
 *   g++ -arch i386 -framework AudioToolbox smash.mm -o smash
 * x86_64 is not supported; it lacks nlist(), and its nlist struct requires
 * a string table index rather than just the symbol name, anyway.
 *
 * Demonstrates that the Mac OS X 10.6+ AudioToolbox's UInt32ToHexString()
 * function triggers a stack protection failure for values greater than
 * 0x10000000.
 *
 * UInt32ToHexString() is a private symbol, which requires jumping through
 * some hoops. Supplying a UInt32 that requires 8 hex digits to represent
 * it suffices to smash the stack. With direct access to the symbol, the
 * demonstration would be as simple as:
 *   char buf[10];
 *   bzero(buf, sizeof(buf));
 *   UInt32 crashing_value = 0x10000000;  // 0x1000 0000
 *   UInt32ToHexString(buf, sizeof(buf) - 1, crashing_value);
 * All values from 0x10000000 through 0xFFFFFFFF (UINT32_MAX) cause
 * UInt32ToHexString() to overrun its buffer.
 */
#import 
#import 
#import 

void *lookup_symbol(const char *name, const char *path);  // helper function

static const char *kUInt32ToHexStringMangledName = 
#if __LP64__
"__ZL17UInt32ToHexStringPcjj";
#else  // 32-bit - i386 or PPC
"__ZL17UInt32ToHexStringPcmm";
#endif
static const char *kAudioToolboxPath
  = "/System/Library/Frameworks/AudioToolbox.framework"
    "/Versions/Current/AudioToolbox";
// The compiled version in the public release of 10.6 was observed to use the
// regparm(3) parameter passing convention.
typedef void __attribute__((regparm(3))) (*UInt32ToHexString_t)(char*, UInt32, UInt32);

int
main(void) {
  // Get the offset into the AudioToolbox binary of UInt32ToHexString.
  UInt32ToHexString_t UInt32ToHexString_p
    = (UInt32ToHexString_t)lookup_symbol(kUInt32ToHexStringMangledName,
                                         kAudioToolboxPath);
  if (UInt32ToHexString_p == NULL) return -1;
  fprintf(stderr, "Found UInt32ToHexString offset: %p\n", UInt32ToHexString_p);
  // Add that offset to the framework's load address.
  // This is done by using dladdr to retrieve information about a
  // public symbol from the AudioToolbox, here, the function CAShow.
  Dl_info info;
  const void *addr_in_image = (const void *)&CAShow;
  if (dladdr(addr_in_image, &info)) {
    fprintf(stderr, "Found image load address: %p\n", info.dli_fbase);
    UInt32ToHexString_p = (UInt32ToHexString_t)((char *)UInt32ToHexString_p
                                                + (ptrdiff_t)info.dli_fbase);
  }
  fprintf(stderr, "Calling UInt32ToHexString_p at %p\n", UInt32ToHexString_p);
  // Prep the arguments to the function call.
  // UInt32ToHexString takes three arguments:
  //   * a char buffer to write to
  //   * the length of that buffer as a UInt32
  //   * the UInt32 value to format into that buffer.
  // The end result is the capital hex digit representation of the supplied
  // value at the end of the buffer; any extra space to the left has ASCII
  // zero characters (0x30) written to it.
  // For example, supplying a 5-byte buffer, length of 5, and the value 0x123
  // results in a buffer looking like "00123".
  char buf[10];  // more than enough space
  bzero(buf, sizeof(buf));
  UInt32 val = 0x10000000;  // val requires 8 hex digits
  //val = 0x1000000;  // requires only 7 hex digits - uncomment to not fail stack check
  UInt32ToHexString_p(buf, sizeof(buf) - 1, val);  // crashes just before returning from here
  printf("val: %X; buf: %s\n", (unsigned int)val, buf);
  return 0;
}

// Returns the address of the symbol |name| in the binary located at |path|.
// Returns NULL on failure.
void *
lookup_symbol(const char *name, const char *path) {
  struct nlist nl[2];
  bzero(&nl, sizeof(nl));
  nl[0].n_un.n_name = const_cast(name);
  void *symbol = NULL;
  if (nlist(path, nl) == -1 || nl[0].n_type == N_UNDF) {
    fprintf(stderr, "nlist(%s, %s) failed\n", path, name);
  } else symbol = (void *)nl[0].n_value;
  return symbol;
}
By jeremyw.sherman at 2009-09-17 01:48:43.989446 (reply...)

Email referenced original bug id of 7226263, but duplicate number in the status line differs (7100093, as reported in the Radar info here).

By jeremyw.sherman at 2009-09-28 15:16:30.558717 (reply...)

Please note: Reports posted here will not necessarily be seen by Apple. All problems should be submitted at bugreport.apple.com before they are posted here. Please only post information for Radars that you have filed yourself, and please do not include Apple confidential information in your posts. Thank you!