Add new interceptors: strlcpy(3) and strlcat(3)

[krytarowski]

Summary

NetBSD ships with strlcpy(3) and strlcat(3), a safe
replacement of strcpy(3) and strcat(3).

Hide both functions under SANITIZER_INTERCEPT_STRLCPY.

Sponsored by <The NetBSD Foundation>

[vitalybuka]

tests?

lib/sanitizer_common/sanitizer_common_interceptors.inc

6514

probably similar as above, but we need to "write" only appended part or we will hide bugs from msan if the beginning of the dst was not initialized.

[krytarowski]

• add tests
• correct strlcat(3)

[vitalybuka]

lib/sanitizer_common/sanitizer_common_interceptors.inc

6639

maybe just?
INTERCEPTOR(SIZE_T, strlcpy, char *dst, char *src, SIZE_T size) {

WRAP(strncpy)(dst, src, size);

}

[krytarowski]

lib/sanitizer_common/sanitizer_common_interceptors.inc

6639

But, these functions differ in behavior. strncpy(3) does not terminate a string always with a NUL-character.

[vitalybuka]

lib/sanitizer_common/sanitizer_common_interceptors.inc

6639

Right, I've already forgot from the last review.

6642

from doc

Also note that strlcpy() and strlcat() only operate on true ``C'' strings.

so we need COMMON_INTERCEPTOR_READ_STRING to enable strict_string_checks

len can't be len > size, so

if len < size
  we need to read len + 1
if len == size
  we need to read len  or size

this probably should be
COMMON_INTERCEPTOR_READ_STRING(ctx, src, MIN(REAL(strnlen)(src, size), size - 1) + 1)

6645

COMMON_INTERCEPTOR_COPY_STRING should be used here, so msan can track origins

6655

same as above, plus I guess we should use
copy_size = size - strlen(dst)
COMMON_INTERCEPTOR_READ_STRING(ctx, src, MIN(REAL(strnlen)(src, copy_size), copy_size - 1) + 1)

6664

Maybe better still user REAL(strlcat)()?
If not, I guess you can just remove src check and just WRAP(strlcpy)() here

6665

COMMON_INTERCEPTOR_COPY_STRING should be used here, so msan can track origins

[krytarowski]

• use COMMON_INTERCEPTOR_COPY_STRING().
• use COMMON_INTERCEPTOR_READ_STRING()

[krytarowski]

Should be done.

lib/sanitizer_common/sanitizer_common_interceptors.inc

6642

"len can't be len > size, so" I don't see this enforced in the specification of the function.

[vitalybuka]

lib/sanitizer_common/sanitizer_common_interceptors.inc

6642

from man strnlen:

The strnlen() function returns the number of characters in the string pointed to by s, excluding the terminating null byte ('\0'), but at most maxlen.

6744

can you remove if (src) branch? it's should be checked by WRAP(strlcpy)

[krytarowski]

lib/sanitizer_common/sanitizer_common_interceptors.inc

6642

I've misunderstood the proposal. I will go for it.

Is there a macro for MIN() or should I define one?

[vitalybuka]

lib/sanitizer_common/sanitizer_common_interceptors.inc

6642

It's actually Min().
It's defined compiler-rt/lib/sanitizer_common/sanitizer_common.h and already used in this file. E.g. INTERCEPTOR(SIZE_T, strnlen

[vitalybuka]

lib/sanitizer_common/sanitizer_common_interceptors.inc

6734

also please put strnlen() into macro:
COMMON_INTERCEPTOR_READ_STRING(ctx, src, ... strnlen() ... )
in this case macro expansion will avoid addition strnlen in case of strict string check.

[krytarowski]

Apply comments from review.

[krytarowski]

lib/sanitizer_common/sanitizer_common_interceptors.inc

6734

I will do it.

[krytarowski]

Apply comments from review.
git-clang-format over tests.

lib/sanitizer_common/sanitizer_common_interceptors.inc

INTERCEPTOR(const char *, user_from_uid, u32 uid, int nouser) {
   void *ctx;
   const char *user;
   COMMON_INTERCEPTOR_ENTER(ctx, user_from_uid, uid, nouser);
   user = REAL(user_from_uid)(uid, nouser);
   if (user)
     COMMON_INTERCEPTOR_WRITE_RANGE(ctx, user, REAL(strlen)(user) + 1);
   return user;
 }
 #define INIT_USER_FROM_UID COMMON_INTERCEPT_FUNCTION(user_from_uid)
 #else
 #define INIT_USER_FROM_UID
 #endif
 
 #if SANITIZER_INTERCEPT_UID_FROM_USER
 INTERCEPTOR(int, uid_from_user, const char *name, u32 *uid) {
   void *ctx;
   int res;

 #else
 #define INIT_GETGROUPLIST
 #endif
 
 #if SANITIZER_INTERCEPT_READLINK
 INTERCEPTOR(SSIZE_T, readlink, const char *path, char *buf, SIZE_T bufsiz) {
   void* ctx;
   COMMON_INTERCEPTOR_ENTER(ctx, readlink, path, buf, bufsiz);
   COMMON_INTERCEPTOR_READ_RANGE(ctx, path, REAL(strlen)(path) + 1);
   SSIZE_T res = REAL(readlink)(path, buf, bufsiz);
   if (res > 0)
     COMMON_INTERCEPTOR_WRITE_RANGE(ctx, buf, res);
   return res;
 }
 
 #define INIT_READLINK COMMON_INTERCEPT_FUNCTION(readlink)
 #else
 #define INIT_READLINK
 #endif
 
 #if SANITIZER_INTERCEPT_READLINKAT
 INTERCEPTOR(SSIZE_T, readlinkat, int dirfd, const char *path, char *buf,
             SIZE_T bufsiz) {
   void* ctx;
   COMMON_INTERCEPTOR_ENTER(ctx, readlinkat, dirfd, path, buf, bufsiz);
   COMMON_INTERCEPTOR_READ_RANGE(ctx, path, REAL(strlen)(path) + 1);
   SSIZE_T res = REAL(readlinkat)(dirfd, path, buf, bufsiz);
   if (res > 0)
     COMMON_INTERCEPTOR_WRITE_RANGE(ctx, buf, res);
   return res;
 }
 
 #define INIT_READLINKAT COMMON_INTERCEPT_FUNCTION(readlinkat)
 #else
 #define INIT_READLINKAT
 #endif
 
 #if SANITIZER_INTERCEPT_NAME_TO_HANDLE_AT
 INTERCEPTOR(int, name_to_handle_at, int dirfd, const char *pathname,
             struct file_handle *handle, int *mount_id, int flags) {
   void* ctx;
   COMMON_INTERCEPTOR_ENTER(ctx, name_to_handle_at, dirfd, pathname, handle,
                            mount_id, flags);
INTERCEPTOR(int, open_by_handle_at, int mount_fd, struct file_handle* handle,
   return REAL(open_by_handle_at)(mount_fd, handle, flags);
 }
 
 #define INIT_OPEN_BY_HANDLE_AT COMMON_INTERCEPT_FUNCTION(open_by_handle_at)
 #else
 #define INIT_OPEN_BY_HANDLE_AT
 #endif
 
+#if SANITIZER_INTERCEPT_STRLCPY
+INTERCEPTOR(SIZE_T, strlcpy, char *dst, char *src, SIZE_T size) {
+  void *ctx;
+  SIZE_T res;
+  COMMON_INTERCEPTOR_ENTER(ctx, strlcpy, dst, src, size);
+  if (src) {
+    // Keep strnlen as macro argument, as macro may ignore it.
+    COMMON_INTERCEPTOR_READ_STRING(ctx, src,
+      Min(REAL(strnlen)(src, size), size - 1) + 1);
+  }
+  res = REAL(strlcpy)(dst, src, size);
+  COMMON_INTERCEPTOR_COPY_STRING(ctx, dst, src, REAL(strlen)(dst) + 1);
+  return res;
+}
+
+INTERCEPTOR(SIZE_T, strlcat, char *dst, char *src, SIZE_T size) {
+  void *ctx;
+  COMMON_INTERCEPTOR_ENTER(ctx, strlcat, dst, src, size);
+  // src is checked in the strlcpy() interceptor
+  if (dst) {
+    SIZE_T len = REAL(strnlen)(dst, size);
+    COMMON_INTERCEPTOR_READ_STRING(ctx, dst, Min(len, size - 1) + 1);
+    // Reuse the rest of the code in the strlcpy() interceptor
+    dst += len;
+    size -= len;
+  }
+  return WRAP(strlcpy)(dst, src, size);
+}
+#define INIT_STRLCPY \
+  COMMON_INTERCEPT_FUNCTION(strlcpy); \
+  COMMON_INTERCEPT_FUNCTION(strlcat);
+#else
+#define INIT_STRLCPY
+#endif
+
 static void InitializeCommonInterceptors() {
   static u64 metadata_mem[sizeof(MetadataHashMap) / sizeof(u64) + 1];
   interceptor_metadata_map = new((void *)&metadata_mem) MetadataHashMap();
 
   INIT_TEXTDOMAIN;
   INIT_STRLEN;
   INIT_STRNLEN;
   INIT_STRNDUP;
static void InitializeCommonInterceptors() {
   INIT_UID_FROM_USER;
   INIT_GROUP_FROM_GID;
   INIT_GID_FROM_GROUP;
   INIT_ACCESS;
   INIT_FACCESSAT;
   INIT_GETGROUPLIST;
   INIT_READLINK;
   INIT_READLINKAT;
-
   INIT_NAME_TO_HANDLE_AT;
   INIT_OPEN_BY_HANDLE_AT;
+  INIT_STRLCPY;
 
 #if SANITIZER_NETBSD
   COMMON_INTERCEPT_FUNCTION(__libc_mutex_lock);
   COMMON_INTERCEPT_FUNCTION(__libc_mutex_unlock);
   COMMON_INTERCEPT_FUNCTION(__libc_thr_setcancelstate);
 #endif
 
   INIT___PRINTF_CHK;
 }

lib/sanitizer_common/sanitizer_platform_interceptors.h

 #define SANITIZER_INTERCEPT_ACCT SI_NETBSD
 #define SANITIZER_INTERCEPT_USER_FROM_UID SI_NETBSD
 #define SANITIZER_INTERCEPT_UID_FROM_USER SI_NETBSD
 #define SANITIZER_INTERCEPT_GROUP_FROM_GID SI_NETBSD
 #define SANITIZER_INTERCEPT_GID_FROM_GROUP SI_NETBSD
 #define SANITIZER_INTERCEPT_ACCESS SI_NETBSD
 #define SANITIZER_INTERCEPT_FACCESSAT SI_NETBSD
 #define SANITIZER_INTERCEPT_GETGROUPLIST SI_NETBSD
+#define SANITIZER_INTERCEPT_STRLCPY SI_NETBSD
 
 #define SANITIZER_INTERCEPT_NAME_TO_HANDLE_AT SI_LINUX_NOT_ANDROID
 #define SANITIZER_INTERCEPT_OPEN_BY_HANDLE_AT SI_LINUX_NOT_ANDROID
 
 #define SANITIZER_INTERCEPT_READLINK SI_POSIX
 #if defined(__ENVIRONMENT_MAC_OS_X_VERSION_MIN_REQUIRED__) && \
     __ENVIRONMENT_MAC_OS_X_VERSION_MIN_REQUIRED__ < 101000
 # define SI_MAC_DEPLOYMENT_BELOW_10_10 1
 #else
 # define SI_MAC_DEPLOYMENT_BELOW_10_10 0
 #endif
 #define SANITIZER_INTERCEPT_READLINKAT \
   (SI_POSIX && !SI_MAC_DEPLOYMENT_BELOW_10_10)
 
 #endif  // #ifndef SANITIZER_PLATFORM_INTERCEPTORS_H

test/sanitizer_common/TestCases/NetBSD/strlcat.cc

+// RUN: %clangxx -O0 -g %s -o %t && %run %t
+
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+
+void test1() {
+  const char src[] = "abc";
+  char dst[7] = {'x', 'y', 'z', 0};
+  size_t len;
+
+  len = strlcat(dst, src, sizeof(dst));
+  printf("%s %zu ", dst, len);
+}
+
+void test2() {
+  const char src[] = "abc";
+  char dst[7] = {0};
+  size_t len;
+
+  len = strlcat(dst, src, sizeof(dst));
+  printf("%s %zu ", dst, len);
+}
+
+void test3() {
+  const char src[] = "abc";
+  char dst[4] = {'x', 'y', 'z', 0};
+  size_t len;
+
+  len = strlcat(dst, src, sizeof(dst));
+  printf("%s %zu ", dst, len);
+}
+
+void test4() {
+  const char src[] = "";
+  char dst[4] = {'x', 'y', 'z', 0};
+  size_t len;
+
+  len = strlcat(dst, src, sizeof(dst));
+  printf("%s %zu\n", dst, len);
+}
+
+int main(void) {
+  test1();
+  test2();
+  test3();
+  test4();
+
+  // CHECK: xyzabc 6 abc 3 xyz 3 xyz 3
+
+  return 0;
+}

test/sanitizer_common/TestCases/NetBSD/strlcpy.cc

+// RUN: %clangxx -O0 -g %s -o %t && %run %t
+
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+
+void test1() {
+  const char src[] = "abc";
+  char dst[7] = {'x', 'y', 'z', 0};
+  size_t len;
+
+  len = strlcpy(dst, src, sizeof(dst));
+  printf("%s %zu ", dst, len);
+}
+
+void test2() {
+  const char src[] = "abc";
+  char dst[7] = {0};
+  size_t len;
+
+  len = strlcat(dst, src, sizeof(dst));
+  printf("%s %zu ", dst, len);
+}
+
+void test3() {
+  const char src[] = "abc";
+  char dst[4] = {'x', 'y', 'z', 0};
+  size_t len;
+
+  len = strlcat(dst, src, sizeof(dst));
+  printf("%s %zu ", dst, len);
+}
+
+void test4() {
+  const char src[] = "";
+  char dst[4] = {'x', 'y', 'z', 0};
+  size_t len;
+
+  len = strlcat(dst, src, sizeof(dst));
+  printf("%s %zu\n", dst, len);
+}
+
+int main(void) {
+  test1();
+  test2();
+  test3();
+  test4();
+
+  // CHECK: abc 3 abc 3 xyz 3  0
+
+  return 0;
+}