[libc] Lay out framework for fuzzing libc functions.

[PaulkaToast]

Summary

Added fuzzing test for strcpy and some documentation related to fuzzing.
This will be the first step in integrating this with oss-fuzz.

[abrachet]

libc/fuzzing/string/strcpy_fuzz.cpp

8

Does oss-fuzz require this to not be mangled?

9–11

No brackets here or the for and its if and also the last if. I think !size might be more common but I don't have a big preference.

15–21

Maybe we will eventually add free standing function templates like those found in <algorithm> so things like this can become cpp::replace(data, data + size, 0, 'a')

16

Capitilize replace

26–28

Is this not assert(strcmp(dest, src)) because you think NDEBUG might be defined for this file?

[sivachandra]

libc/fuzzing/string/strcpy_fuzz.cpp

8

Just a few high level comments for now. Might have more later.

Avoid using malloc/memcpy/abort:

• Return a non-zero value instead of abort.
• Instead of malloc/memcpy/free, split the input data into two parts deterministic-ally. Say, use the first N bytes to determine the size of the first part.
• If you think a generic data provider makes sense, then we should probably build one for our use. For example, like this: https://github.com/llvm/llvm-project/blob/master/compiler-rt/include/fuzzer/FuzzedDataProvider.h

[PaulkaToast]

libc/fuzzing/string/strcpy_fuzz.cpp

8

Yes, LibFuzzer and indirectly oss-fuzz requires symbols to be unmangled.

8

Just to address the first comment. Non-zero returns are reserved by LibFuzzer. The usage to indicate fault is to crash the program.

26–28

oss-fuzz compiles with optimization -o3 enabled. Does NDEBUG get defined with that level of optimization? If it does then assert will not crash the fuzzer as expected.

[abrachet]

libc/fuzzing/string/strcpy_fuzz.cpp

26–28

@sivachandra said to avoid abort, so that would mean avoid assert too, we could change this abort to __builtin_trap.

FWIW https://godbolt.org/z/khdC4E

[sivachandra]

libc/fuzzing/string/strcpy_fuzz.cpp

26–28

I am not really against using abort but against including stdlib.h. So, we can create an abort wrapper in utils/CPP which allows us to avoid including stdlib.h.

But, if exit is disallowed in a fuzz target, is abort OK? FWIW, libcxx's fuzz tests seem to prefer assert as @abrachet suggested.

Also, asking for my own knowledge: Should one care about correctness in a fuzz test? Correctness is important of course, but is that the goal of a fuzz test?

[sivachandra]

libc/fuzzing/string/strcpy_fuzz.cpp

26–28

Just correcting myself: If we conclude we need abort/assert, then we should put their wrappers in a fuzzing specific util-library and not in utils/CPP.

[MaskRay]

libc/fuzzing/string/strcpy_fuzz.cpp

13

If malloc returns NULL, return 0, otherwise when the system is under high memory pressure, the code may incorrectly trigger a crash.

14

Placing malloc in the function LLVMFuzzerTestOneInput may make tests run slowly.

26

Braces around a single statement are not common in LLVM code. I think Google code tends to have more braces because:

% cat a.c
int main() {
  if (strcmp(dest, src) != 0)
    abort();
}
% clang-format --style=Google a.c
int main() {
  if (strcmp(dest, src) != 0) abort();
}

Many consider if (...) ... on the same line strange. LLVM style does not have the problem.

[PaulkaToast]

libc/fuzzing/string/strcpy_fuzz.cpp

14

The test case is rather simple so it runs sufficiently fast about 150k+ executions per second on one of my machine's cores.

Since we cannot modify the fuzzer input data the only alternative would be using a static buffer, however that introduces a size constraint and we could miss a bug with bigger strings.

26

Ah, thank you!

[abrachet]

libc/fuzzing/string/strcpy_fuzz.cpp

14

nit: make data const uint8_t* then.

[PaulkaToast]

libc/fuzzing/string/strcpy_fuzz.cpp

26–28

LibFuzzer's documentation makes use of __builtin_trap so I'm replacing abort calls.

As for correctness note, I believe that that there is a usefulness to having correctness also be a goal of the fuzz test. Here are some reason for this practice we might consider?

[PaulkaToast]

Removed most dependencies on system libc headers and integrated changes requested by Kostya.

[sivachandra]

I terribly back-logged. I can fix up the target naming scheme issues later. Just one nit to fix and you can land it.

libc/docs/fuzzing.rst

1

Limit to 80-char line widths.

[abrachet]

libc/fuzzing/string/strcpy_fuzz.cpp

1

We would probably want a header comment here? Also please run clang-format

5

validate input -> Validate input. The same for the rest of the comments

16

Couldn't this just be from i = 0 to size?

[PaulkaToast]

libc/fuzzing/string/strcpy_fuzz.cpp

16

The length of the string that strcpy copies may not be the same as the size of the fuzzing input due to null-terminators appearing at random in data.

[abrachet]

libc/fuzzing/string/strcpy_fuzz.cpp

16

Then if it is completely random this if (data[size - 1] != '\0') return 0; will end the test 255/256 times, no?

Also without removing the 0's like before from the input then the average length will be just 256 then. Is this a problem? Or perhaps a better question is it a smaller problem than the previously raised concerned that the extra allocation was too costly?

23

Should we be failing when the system can't allocate memory this isn't __llvm_libc::strcpy's fault.

[PaulkaToast]

libc/fuzzing/string/strcpy_fuzz.cpp

23

This was discussed with Kotsya in an offline meeting. It is extremely unlikely that the system would be out of memory for the relatively small sizes (by default under 4k bytes though oss-fuzz tests with larger inputs). So a failure here probably indicates something going wrong with the memory allocator which is worth catching as well.

[PaulkaToast]

libc/fuzzing/string/strcpy_fuzz.cpp

16

Apologies, it wouldn't be completely random, the fuzzer is coverage guided so it'll learn that null-terminated strings are what we expect and it should then provide that more often. This was explained to me offline and it seems that modifying the input isn't necessarily needed and we should leave it up to the fuzzer.

[abrachet]

libc/fuzzing/string/strcpy_fuzz.cpp

16

I see. Sorry about the ignorance on fuzzers!

[PaulkaToast]

libc/fuzzing/string/strcpy_fuzz.cpp

16

No worries, I was ignorant too until it was explained to me. (:

[sivachandra]

libc/fuzzing/string/strcpy_fuzz.cpp

1

Ah, thanks for catching the missing license header.

libc/CMakeLists.txt

 add_subdirectory(include)
 add_subdirectory(utils)
 
 # The lib and test directories are added at the very end as tests
 # and libraries potentially draw from the components present in all
 # of the other directories.
 add_subdirectory(lib)
 add_subdirectory(test)
+add_subdirectory(fuzzing)

libc/cmake/modules/LLVMLibCRules.cmake

 #      SRCS  <list of .cpp files for the test>
 #      HDRS  <list of .h files for the test>
 #      DEPENDS <list of dependencies>
 #    )
 function(add_libc_unittest target_name)
   if(NOT LLVM_INCLUDE_TESTS)
     return()
   endif()
   
   cmake_parse_arguments(
     "LIBC_UNITTEST"
     "" # No optional arguments
     "SUITE" # Single value arguments
     "SRCS;HDRS;DEPENDS" # Multi-value arguments
     ${ARGN}
   )
   if(NOT LIBC_UNITTEST_SRCS)
function(add_libc_unittest target_name)
   endif()
 endfunction(add_libc_unittest)
 
 function(add_libc_testsuite suite_name)
   add_custom_target(${suite_name})
   add_dependencies(check-libc ${suite_name})
 endfunction(add_libc_testsuite)
 
+# Rule to add a fuzzer test.
+# Usage
+#    add_libc_fuzzer(
+#      <target name>
+#      SRCS  <list of .cpp files for the test>
+#      HDRS  <list of .h files for the test>
+#      DEPENDS <list of dependencies>
+#    )
+function(add_libc_fuzzer target_name)
+  cmake_parse_arguments(
+    "LIBC_FUZZER"
+    "" # No optional arguments
+    "" # Single value arguments
+    "SRCS;HDRS;DEPENDS" # Multi-value arguments
+    ${ARGN}
+  )
+  if(NOT LIBC_FUZZER_SRCS)
+    message(FATAL_ERROR "'add_libc_fuzzer' target requires a SRCS list of .cpp files.")
+  endif()
+  if(NOT LIBC_FUZZER_DEPENDS)
+    message(FATAL_ERROR "'add_libc_fuzzer' target requires a DEPENDS list of 'add_entrypoint_object' targets.")
+  endif()
+
+  set(library_deps "")
+  foreach(dep IN LISTS LIBC_FUZZER_DEPENDS)
+    get_target_property(dep_type ${dep} "TARGET_TYPE")
+    if (dep_type)
+      string(COMPARE EQUAL ${dep_type} ${ENTRYPOINT_OBJ_TARGET_TYPE} dep_is_entrypoint)
+      if(dep_is_entrypoint)
+        get_target_property(obj_file ${dep} "OBJECT_FILE_RAW")
+        list(APPEND library_deps ${obj_file})
+        continue()
+      endif()
+    endif()
+    # TODO: Check if the dep is a normal CMake library target. If yes, then add it
+    # to the list of library_deps.
+  endforeach(dep)
+
+  add_executable(
+    ${target_name}
+    EXCLUDE_FROM_ALL
+    ${LIBC_FUZZER_SRCS}
+    ${LIBC_FUZZER_HDRS}
+  )
+  target_include_directories(
+    ${target_name}
+    PRIVATE
+      ${LIBC_SOURCE_DIR}
+      ${LIBC_BUILD_DIR}
+      ${LIBC_BUILD_DIR}/include
+  )
+
+  if(library_deps)
+    target_link_libraries(${target_name} PRIVATE ${library_deps})
+  endif()
+
+  set_target_properties(${target_name} PROPERTIES RUNTIME_OUTPUT_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR})
+
+  add_dependencies(
+    ${target_name}
+    ${LIBC_FUZZER_DEPENDS}
+  )
+  add_dependencies(libc-fuzzer ${target_name})
+endfunction(add_libc_fuzzer)
+
 # Rule to add header only libraries.
 # Usage
 #    add_header_library(
 #      <target name>
 #      HDRS  <list of .h files part of the library>
 #      DEPENDS <list of dependencies>
 #    )
 function(add_header_library target_name)

libc/docs/fuzzing.rst

+Fuzzing for LLVM-libc
+---------------------
+
+Fuzzing tests are used to ensure quality and security of LLVM-libc implementations. 
+
+Each fuzzing test lives under the fuzzing directory in a subdirectory corresponding with the src layout. 
+
+Currently we use system libc for functions that have yet to be implemented, however as they are implemented the fuzzers will be changed to use our implementation to increase coverage for testing. 
+
+Fuzzers will be run on `oss-fuzz <https://github.com/google/oss-fuzz>`_ and the check-libc target will ensure that they build correctly. 

libc/docs/source_layout.rst

 LLVM-libc Source Tree Layout
 ============================
 
 At the top-level, LLVM-libc source tree is organized in to the following
 directories::
 
    + libc
         - cmake
         - docs
+        - fuzzing
         - include
         - lib
         - loader
         - src
         - test
         + utils
             - build_scripts
             - testing
         - www
 
 Each of these directories is explained in detail below.
 
 The ``cmake`` directory
 -----------------------
 
 The ``cmake`` directory contains the implementations of LLVM-libc's CMake build
 rules.
 
 The ``docs`` directory
 ----------------------
 
 The ``docs`` directory contains design docs and also informative documents like
 this document on source layout.
 
+The ``fuzzing`` directory
+----------------------
+
+This directory contains fuzzing tests for the various components of llvm-libc. The
+directory structure within this directory mirrors the directory structure of the
+top-level ``libc`` directory itself. For more details, see :doc:`fuzzing`.
+
 The ``include`` directory
 -------------------------
 
 The ``include`` directory contains:
 
 1. Self contained public header files - These are header files which are
    already in the form that get installed when LLVM-libc is installed on a user's
    computer.

 ``crt1.o`` etc.
 
 The ``src`` directory
 ---------------------
 
 This directory contains the implementations of the llvm-libc entrypoints. It is
 further organized as follows:
 
-1. There is a toplevel CMakeLists.txt file.
+1. There is a top-level CMakeLists.txt file.
 2. For every public header file provided by llvm-libc, there exists a
    corresponding directory in the ``src`` directory. The name of the directory
    is same as the base name of the header file. For example, the directory
    corresponding to the public ``math.h`` header file is named ``math``. The
    implementation standard document explains more about the *header*
    directories.
 
 The ``test`` directory

libc/fuzzing/CMakeLists.txt

+set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -fsanitize=fuzzer")
+add_custom_target(libc-fuzzer)
+add_dependencies(check-libc libc-fuzzer)
+
+add_subdirectory(string)

libc/fuzzing/string/CMakeLists.txt

+add_libc_fuzzer(
+  strcpy_fuzz
+  SRCS
+    strcpy_fuzz.cpp
+  DEPENDS
+    strcpy
+)

libc/fuzzing/string/strcpy_fuzz.cpp

+#include "src/string/strcpy.h"
+#include <stdint.h>
+
+extern "C" int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size) {
+  // validate input
+  if (!size) return 0;
+  if (data[size - 1] != '\0') return 0;
+  const char * src = (const char *)data;
+
+  char *dest = new char[size];
+  if (!dest) __builtin_trap();
+
+  __llvm_libc::strcpy(dest, src);
+
+  size_t i;
+  for (i = 0; src[i] != '\0'; i++){
+    // ensure correctness of strcpy
+    if (dest[i] != src[i]) __builtin_trap();
+  }
+  // ensure strcpy null terminates dest
+  if (dest[i] != src[i]) __builtin_trap();
+
+  delete[] dest;
+  
+  return 0;
+}