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

6489

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

6470

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

6470

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

6470

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

6473

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)

6476

COMMON_INTERCEPTOR_COPY_STRING should be used here, so msan can track origins

6486

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)

6495

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

6496

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

6473

"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

6473

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.

6483

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

[krytarowski]

lib/sanitizer_common/sanitizer_common_interceptors.inc

6473

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

6473

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

6473

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

6473

I will do it.

[krytarowski]

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

lib/sanitizer_common/sanitizer_common_interceptors.inc

 }
 #define INIT_WCSCAT                  \
   COMMON_INTERCEPT_FUNCTION(wcscat); \
   COMMON_INTERCEPT_FUNCTION(wcsncat);
 #else
 #define INIT_WCSCAT
 #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) {
+    SIZE_T len = REAL(strnlen)(src, size);
+    COMMON_INTERCEPTOR_READ_RANGE(ctx, src, len >= size ? size : len + 1);
+  }
+  res = REAL(strlcpy)(dst, src, size);
+  COMMON_INTERCEPTOR_WRITE_RANGE(ctx, dst, REAL(strlen)(dst) + 1);
+  return res;
+}
+
+INTERCEPTOR(SIZE_T, strlcat, char *dst, char *src, SIZE_T size) {
+  void *ctx;
+  SIZE_T res;
+  COMMON_INTERCEPTOR_ENTER(ctx, strlcat, dst, src, size);
+  if (src) {
+    SIZE_T len = REAL(strnlen)(src, size);
+    COMMON_INTERCEPTOR_READ_RANGE(ctx, src, len >= size ? size : len + 1);
+  }
+  res = REAL(strlcat)(dst, src, size);
+  COMMON_INTERCEPTOR_WRITE_RANGE(ctx, dst, REAL(strlen)(dst) + 1);
+  return res;
+}
+#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___LXSTAT;
   INIT___LXSTAT64;
   // FIXME: add other *stat interceptors.
   INIT_UTMP;
   INIT_UTMPX;
   INIT_GETLOADAVG;
   INIT_WCSLEN;
   INIT_WCSCAT;
+  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_CFREE \
   (!SI_FREEBSD && !SI_MAC && !SI_NETBSD && SI_NOT_FUCHSIA)
 #define SANITIZER_INTERCEPT_ALIGNED_ALLOC (!SI_MAC)
 #define SANITIZER_INTERCEPT_MALLOC_USABLE_SIZE (!SI_MAC)
 #define SANITIZER_INTERCEPT_MCHECK_MPROBE SI_LINUX_NOT_ANDROID
 #define SANITIZER_INTERCEPT_WCSCAT SI_POSIX
 #define SANITIZER_INTERCEPT_SIGNAL_AND_SIGACTION (!SI_WINDOWS && SI_NOT_FUCHSIA)
 #define SANITIZER_INTERCEPT_BSD_SIGNAL SI_ANDROID
 
+#define SANITIZER_INTERCEPT_STRLCPY SI_NETBSD
+
 #endif  // #ifndef SANITIZER_PLATFORM_INTERCEPTORS_H