Skip to content

[win/asan] Ensure errno gets set correctly for strtol #109258

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

Merged
merged 1 commit into from
Sep 22, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 26 additions & 1 deletion compiler-rt/lib/asan/asan_interceptors.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -650,9 +650,34 @@ static ALWAYS_INLINE auto StrtolImpl(void *ctx, Fn real, const char *nptr,
return StrtolImpl(ctx, REAL(func), nptr, endptr, base); \
}

INTERCEPTOR_STRTO_BASE(long, strtol)
INTERCEPTOR_STRTO_BASE(long long, strtoll)

# if SANITIZER_WINDOWS
INTERCEPTOR(long, strtol, const char *nptr, char **endptr, int base) {
// REAL(strtol) may be ntdll!strtol, which doesn't set errno. Instead,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Long term, we should stop patching ntdll for anything other than the purpose of intercepting heap allocation. For maintenance reasons, it's best not to touch the string routines in the minicrt inside of ntdll.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ack.

// call REAL(strtoll) and do the range check ourselves.
COMPILER_CHECK(sizeof(long) == sizeof(u32));

void *ctx;
ASAN_INTERCEPTOR_ENTER(ctx, strtol);
AsanInitFromRtl();

long long result = StrtolImpl(ctx, REAL(strtoll), nptr, endptr, base);

if (result > INT32_MAX) {
errno = errno_ERANGE;
return INT32_MAX;
}
if (result < INT32_MIN) {
errno = errno_ERANGE;
return INT32_MIN;
}
return (long)result;
}
# else
INTERCEPTOR_STRTO_BASE(long, strtol)
# endif

# if SANITIZER_GLIBC
INTERCEPTOR_STRTO_BASE(long, __isoc23_strtol)
INTERCEPTOR_STRTO_BASE(long long, __isoc23_strtoll)
Expand Down
5 changes: 4 additions & 1 deletion compiler-rt/lib/asan/asan_win_static_runtime_thunk.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -63,10 +63,13 @@ INTERCEPT_LIBRARY_FUNCTION_ASAN(strpbrk);
INTERCEPT_LIBRARY_FUNCTION_ASAN(strspn);
INTERCEPT_LIBRARY_FUNCTION_ASAN(strstr);
INTERCEPT_LIBRARY_FUNCTION_ASAN(strtok);
INTERCEPT_LIBRARY_FUNCTION_ASAN(strtol);
INTERCEPT_LIBRARY_FUNCTION_ASAN(wcslen);
INTERCEPT_LIBRARY_FUNCTION_ASAN(wcsnlen);

// Note: Don't intercept strtol(l). They are supposed to set errno for out-of-
// range values, but since the ASan runtime is linked against the dynamic CRT,
// its errno is different from the one in the current module.

# if defined(_MSC_VER) && !defined(__clang__)
# pragma warning(pop)
# endif
Expand Down
24 changes: 24 additions & 0 deletions compiler-rt/lib/asan/tests/asan_str_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -638,3 +638,27 @@ TEST(AddressSanitizer, StrtolOOBTest) {
RunStrtolOOBTest(&CallStrtol);
}
#endif

TEST(AddressSanitizer, StrtolOverflow) {
if (sizeof(long) == 4) {
// Check that errno gets set correctly on 32-bit strtol overflow.
long res;
errno = 0;
res = Ident(strtol("2147483647", NULL, 0));
EXPECT_EQ(errno, 0);
EXPECT_EQ(res, 2147483647);

res = Ident(strtol("2147483648", NULL, 0));
EXPECT_EQ(errno, ERANGE);
EXPECT_EQ(res, 2147483647);

errno = 0;
res = Ident(strtol("-2147483648", NULL, 0));
EXPECT_EQ(errno, 0);
EXPECT_EQ(res, -2147483648);

res = Ident(strtol("-2147483649", NULL, 0));
EXPECT_EQ(errno, ERANGE);
EXPECT_EQ(res, -2147483648);
}
}
1 change: 1 addition & 0 deletions compiler-rt/lib/sanitizer_common/sanitizer_errno.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ namespace __sanitizer {
COMPILER_CHECK(errno_ENOMEM == ENOMEM);
COMPILER_CHECK(errno_EBUSY == EBUSY);
COMPILER_CHECK(errno_EINVAL == EINVAL);
COMPILER_CHECK(errno_ERANGE == ERANGE);

// EOWNERDEAD is not present in some older platforms.
#if defined(EOWNERDEAD)
Expand Down
1 change: 1 addition & 0 deletions compiler-rt/lib/sanitizer_common/sanitizer_errno_codes.h
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ namespace __sanitizer {
#define errno_ENOMEM 12
#define errno_EBUSY 16
#define errno_EINVAL 22
#define errno_ERANGE 34
#define errno_ENAMETOOLONG 36
#define errno_ENOSYS 38

Expand Down
3 changes: 3 additions & 0 deletions compiler-rt/test/asan/TestCases/strtol_strict.c
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,9 @@
// RUN: %env_asan_opts=strict_string_checks=true not %run %t test7 2>&1 | FileCheck %s --check-prefix=CHECK7
// REQUIRES: shadow-scale-3

// On Windows, strtol cannot be intercepted when statically linked against the CRT.
// UNSUPPORTED: win32-static-asan

#include <assert.h>
#include <stdlib.h>
#include <string.h>
Expand Down
Loading