OWASP: Buffer Overflow Vulnerabilities

Describe the bug

My organization has vulnerability pentests performed and the Bugsnag Objective-C code has been flagged for Buffer Overflow vulnerabilities.

More info on these types of vulnerabilities can be found here and here.

Steps to reproduce

bugsnag-cocoa git:(master) find ./Bugsnag | xargs grep -s "_sscanf\|_fopen\|_vsnprintf\|_strlen\|_memcpy\|_strncpy\|_stat(\|_malloc\|calloc"
./Bugsnag/Payload/BugsnagThread.m:    struct backtrace_t *backtraces = calloc(threadCount, sizeof(struct backtrace_t));
./Bugsnag/Payload/BugsnagThread.m:    threadStates = calloc(threadCount, sizeof(*threadStates));
./Bugsnag/KSCrash/Source/KSCrash/Recording/BSG_KSCrashDoctor.m:        if ([symbolName isEqualToString:@"szone_malloc_should_clear"]) {
./Bugsnag/KSCrash/Source/KSCrash/Recording/Tools/BSG_KSMachHeaders.c:    BSG_Mach_Header_Info *newImage = calloc(1, sizeof(BSG_Mach_Header_Info));
./Bugsnag/KSCrash/Source/KSCrash/Recording/Tools/BSG_KSLogger.c:    int len = bsg_vsnprintf(buf, size, fmt, args);
./Bugsnag/KSCrash/Source/KSCrash/Recording/Tools/BSG_KSLogger.c:        int msglen = bsg_vsnprintf(buf + len, max, fmt, args);
./Bugsnag/Breadcrumbs/BugsnagBreadcrumbs.m:    struct bsg_breadcrumb_list_item *newItem = calloc(1, sizeof(struct bsg_breadcrumb_list_item) + data.length + 1);
./Bugsnag/Helpers/BSGUtils.m:    if (data.length && (buffer = calloc(1, data.length + 1))) {
./Bugsnag/Helpers/stb_sprintf.h:int stbsp_vsnprintf( char * buf, int count, char const * fmt, va_list va )
./Bugsnag/Helpers/stb_sprintf.h:  Convert a va_list arg list into a buffer.  stbsp_vsnprintf always returns
./Bugsnag/Helpers/stb_sprintf.h:static STBSP__ASAN stbsp__uint32 stbsp__strlen_limited(char const *s, stbsp__uint32 limit)
./Bugsnag/Helpers/stb_sprintf.h:         l = stbsp__strlen_limited(s, (pr >= 0) ? pr : ~0u);

Environment

Bugsnag version: 6.27.2

Hi @mdcarter1

We do not believe these findings show any vulnerability in our code base. This is a breakdown of why:

These incorrectly show use of _vsnprintf and _strlen. We're in fact calling bsg_vsnprintf and stbsp__strlen_limited, which are different implementations that don't have the same vulnerabilities:

./Bugsnag/KSCrash/Source/KSCrash/Recording/Tools/BSG_KSLogger.c:    int len = bsg_vsnprintf(buf, size, fmt, args);
./Bugsnag/KSCrash/Source/KSCrash/Recording/Tools/BSG_KSLogger.c:        int msglen = bsg_vsnprintf(buf + len, max, fmt, args);
./Bugsnag/Helpers/stb_sprintf.h:int stbsp_vsnprintf( char * buf, int count, char const * fmt, va_list va )
./Bugsnag/Helpers/stb_sprintf.h:  Convert a va_list arg list into a buffer.  stbsp_vsnprintf always returns
./Bugsnag/Helpers/stb_sprintf.h:static STBSP__ASAN stbsp__uint32 stbsp__strlen_limited(char const *s, stbsp__uint32 limit)
./Bugsnag/Helpers/stb_sprintf.h:         l = stbsp__limited(s, (pr >= 0) ? pr : ~0u);

This is just a string, not a _malloc call:

./Bugsnag/KSCrash/Source/KSCrash/Recording/BSG_KSCrashDoctor.m:        if ([symbolName isEqualToString:@"szone_malloc_should_clear"]) {

These are calls to calloc, and so CVE-2006-7252 is a consideration. However, the vulnerability only exists if the number of blocks requested could overflow (and thus be much smaller than desired). For all these uses, the number of requested blocks is fixed (i.e. 1):

./Bugsnag/KSCrash/Source/KSCrash/Recording/Tools/BSG_KSMachHeaders.c:    BSG_Mach_Header_Info *newImage = calloc(1, sizeof(BSG_Mach_Header_Info));
./Bugsnag/Breadcrumbs/BugsnagBreadcrumbs.m:    struct bsg_breadcrumb_list_item *newItem = calloc(1, sizeof(struct bsg_breadcrumb_list_item) + data.length + 1);
./Bugsnag/Helpers/BSGUtils.m:    if (data.length && (buffer = calloc(1, data.length + 1))) {

Or is not feasible for them to get anywhere near high enough for an overflow – threadCount on a device would need to be over 15372286728091293 for this to be an issue. Also, the input is not in any way controllable by a user, so could not be exploited.

./Bugsnag/Payload/BugsnagThread.m:    struct backtrace_t *backtraces = calloc(threadCount, sizeof(struct backtrace_t));
./Bugsnag/Payload/BugsnagThread.m:    threadStates = calloc(threadCount, sizeof(*threadStates));
./Bugsnag/KSCrash/Source/KSCrash/Recording/Tools/BSG_KSMachHeaders.c:    BSG_Mach_Header_Info *newImage = calloc(1, sizeof(BSG_Mach_Header_Info));
./Bugsnag/Breadcrumbs/BugsnagBreadcrumbs.m:    struct bsg_breadcrumb_list_item *newItem = calloc(1, sizeof(struct bsg_breadcrumb_list_item) + data.length + 1);
./Bugsnag/Helpers/BSGUtils.m:    if (data.length && (buffer = calloc(1, data.length + 1))) {

bugsnag / bugsnag-cocoa