[Sanitizers][Darwin] Intercept CCRandomGenerateBytes

[devnexen]

Summary

• Intercept the native apple (both iOs and macOS) which is even allowed on App Store (contrary to getentropy for example), thus checking its proper usage in code consumers.

[yln]

Hi David, thanks for the patch for this previously unintercepted API. I am happy with the interceptor implementation itself; it follows our "recipe" nicely! :)

I have two small asks:

• In the commit message, can you explain which issue (crash, false positive) for which sanitizer this is addressing?
• Can we write a test that shows this issue? I think the current test only shows that we don't crash, right?

compiler-rt/lib/sanitizer_common/sanitizer_common_interceptors.inc

7246

Nitpick: can we use the parameter names "bytes" and "count" from the header declaration?

CCRNGStatus CCRandomGenerateBytes(void *bytes, size_t count)
__OSX_AVAILABLE_STARTING(__MAC_10_10, __IPHONE_8_0);

compiler-rt/test/sanitizer_common/TestCases/Darwin/ccrandomgeneratebytes.c

1

Are we testing here simply that we don't crash in our interceptor? Can we add a comment what we intend to test here.

4

I don't think this will work as intended. We would try to compile and run an empty file.
However, I think no one is actually testing on macOS version < 10.10, so just omitting this guard should be fine.

12

I don't think we will ever return "false", i.e., failing the test here?

success -> n = 0
failure -> n = 1
return n <= 0   // n is never <= 0, right?!

Maybe just return the return value of CCRandomGenerateBytes() (assuming kCCSuccess is 0)?

[devnexen]

Update unit test case.

[vitalybuka]

compiler-rt/lib/sanitizer_common/sanitizer_common_interceptors.inc

9836–9845

Why this renaming? As it it matches https://man7.org/linux/man-pages/man2/getrandom.2.html
Either way it looks unrelated to the patch.

[vitalybuka]

It's titled "Intercept CCRandomGenerateBytes", but I don't see changes in interceptors, only new test.

[devnexen]

compiler-rt/lib/sanitizer_common/sanitizer_common_interceptors.inc

9836–9845

My bad wrong lines.

[yln]

Hi David,

Before I approve this can we do the following?

• In the commit message, can you explain which issue (crash, false positive) for which sanitizer this is addressing?
• Can we write a test that shows this issue, i.e., a test that fails without the added interceptor? (Maybe this test needs to be for a specific sanitizer instead of sanitizer_common?)

Note: my understanding is that the added test is "green" even without the new interceptor. Ignore my second point if that is not the case.

[devnexen]

you re right after second thoughts it does not bring anything on the table.

compiler-rt/lib/sanitizer_common/sanitizer_common_interceptors.inc

   COMMON_INTERCEPT_FUNCTION(sl_init); \
   COMMON_INTERCEPT_FUNCTION(sl_add);  \
   COMMON_INTERCEPT_FUNCTION(sl_find); \
   COMMON_INTERCEPT_FUNCTION(sl_free);
 #else
 #define INIT_SL_INIT
 #endif
 
 #if SANITIZER_INTERCEPT_GETRANDOM
-INTERCEPTOR(SSIZE_T, getrandom, void *buf, SIZE_T buflen, unsigned int flags) {
+INTERCEPTOR(SSIZE_T, getrandom, void *bytes, SIZE_T count, unsigned int flags) {
   void *ctx;
-  COMMON_INTERCEPTOR_ENTER(ctx, getrandom, buf, buflen, flags);
-  SSIZE_T n = REAL(getrandom)(buf, buflen, flags);
+  COMMON_INTERCEPTOR_ENTER(ctx, getrandom, bytes, count, flags);
+  SSIZE_T n = REAL(getrandom)(bytes, count, flags);
   if (n > 0) {
-    COMMON_INTERCEPTOR_WRITE_RANGE(ctx, buf, n);
+    COMMON_INTERCEPTOR_WRITE_RANGE(ctx, bytes, n);
   }
   return n;
 }
 #define INIT_GETRANDOM COMMON_INTERCEPT_FUNCTION(getrandom)
 #else
 #define INIT_GETRANDOM
 #endif
 
 #if SANITIZER_INTERCEPT_CRYPT
 INTERCEPTOR(char *, crypt, char *key, char *salt) {
   void *ctx;

compiler-rt/test/sanitizer_common/TestCases/Darwin/ccrandomgeneratebytes.c

+// We try to ensure the api interceptor functions properly.
+//
 // RUN: %clang -O2 %s -o %t && %run %t
 
-#include <Availability.h>
-#if __ENVIRONMENT_MAC_OS_X_VERSION_MIN_REQUIRED__ > 101000
 #include <CommonCrypto/CommonRandom.h>
+#include <stdio.h>
+#include <inttypes.h>
 
 int main() {
   char buf[16];
-  int n = 0;
-  if (CCRandomGenerateBytes(buf, sizeof(buf)) != kCCSuccess)
-    n = 1;
-  return n <= 0;
+  int n = !(CCRandomGenerateBytes(buf, sizeof(buf)) == kCCSuccess);
+  if (n == 0) {
+    printf("CCRandomGenerateBytes\n");
+    for (int i = 0; i <sizeof(buf); i++)
+      printf("%" PRIx8, buf[i]);
+    printf("\n");
+  }
+  return n;
+  // CHECK: CCRandomGenerateBytes
+  // CHECK: '{{.*}}'
 }
-#endif