[PATCH] D42061: Add new interceptors: strlcpy(3) and strlcat(3)

[Vitaly Buka via Phabricator via llvm-commits]

vitalybuka added inline comments.


================
Comment at: lib/sanitizer_common/sanitizer_common_interceptors.inc:6615
+  SIZE_T res;
+  COMMON_INTERCEPTOR_ENTER(ctx, strlcpy, dst, src, size);
+  if (src) {
----------------
krytarowski wrote:
> vitalybuka wrote:
> > maybe just?
> > INTERCEPTOR(SIZE_T, strlcpy, char *dst, char *src, SIZE_T size) {
> >   WRAP(strncpy)(dst, src, size);
> > }
> > 
> > 
> But, these functions differ in behavior. strncpy(3) does not terminate a string always with a NUL-character.
Right, I've already forgot from the last review.



================
Comment at: lib/sanitizer_common/sanitizer_common_interceptors.inc:6618
+    SIZE_T len = REAL(strnlen)(src, size);
+    COMMON_INTERCEPTOR_READ_RANGE(ctx, src, len >= size ? size : len + 1);
+  }
----------------
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)


================
Comment at: lib/sanitizer_common/sanitizer_common_interceptors.inc:6621
+  res = REAL(strlcpy)(dst, src, size);
+  COMMON_INTERCEPTOR_WRITE_RANGE(ctx, dst, REAL(strlen)(dst) + 1);
+  return res;
----------------
COMMON_INTERCEPTOR_COPY_STRING should be used here, so msan can track origins


================
Comment at: lib/sanitizer_common/sanitizer_common_interceptors.inc:6631
+    SIZE_T len = REAL(strnlen)(src, size);
+    COMMON_INTERCEPTOR_READ_RANGE(ctx, src, len >= size ? size : len + 1);
+  }
----------------
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)


================
Comment at: lib/sanitizer_common/sanitizer_common_interceptors.inc:6640
+  }
+  res = REAL(strlcpy)(dst, src, size);
+  COMMON_INTERCEPTOR_WRITE_RANGE(ctx, dst, REAL(strlen)(dst) + 1);
----------------
Maybe better still user REAL(strlcat)()?
If not, I guess you can just remove src check and just WRAP(strlcpy)() here


================
Comment at: lib/sanitizer_common/sanitizer_common_interceptors.inc:6641
+  res = REAL(strlcpy)(dst, src, size);
+  COMMON_INTERCEPTOR_WRITE_RANGE(ctx, dst, REAL(strlen)(dst) + 1);
+  return res;
----------------
COMMON_INTERCEPTOR_COPY_STRING should be used here, so msan can track origins


Repository:
  rL LLVM

https://reviews.llvm.org/D42061