Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Try2 increase code coverage #290

Open
wants to merge 8 commits into
base: master
Choose a base branch
from
Open

Try2 increase code coverage #290

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
74de349
Add padding frame fuzzing. Note it corresponds to handle_padding_frame
sescandor Mar 2, 2020
634179c
Add fuzzing for address token encryption. TODO: add token decryption …
sescandor Mar 2, 2020
34c2e5f
Update the cc being used to build the exe
sescandor Mar 2, 2020
926d85e
set_cid undeclared usage is now fixed. One last one to go
sescandor Mar 4, 2020
8b854d5
builds quicly-fuzzer-address-token. TODO: clean this up
sescandor Mar 5, 2020
ea270b7
Cleanup of address_token.cc
sescandor Mar 5, 2020
605ceed
Clean up fuzz target; Fix heap buffer overflow, when size of the give…
sescandor Mar 6, 2020
20445ea
Clean up CMakeLists; heed compiler warning and initialize ret
sescandor Mar 6, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 28 additions & 6 deletions CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,30 +94,52 @@ IF (CMAKE_SYSTEM_NAME STREQUAL "Linux")
SET(CMAKE_C_FLAGS "-D_GNU_SOURCE -pthread ${CMAKE_C_FLAGS}")
ENDIF ()


IF (BUILD_FUZZER)
MESSAGE(STATUS "************* Making the fuzzer")
MESSAGE(STATUS "************* Making the packet fuzzer")
SET(FUZZER_EXE "quicly-fuzzer-packet")
SET(FUZZER_CC "fuzz/packet.cc")

IF(NOT CMAKE_CXX_COMPILER_ID STREQUAL "Clang")
MESSAGE(FATAL_ERROR "The fuzzer needs clang as a compiler")
ENDIF()
ADD_EXECUTABLE(quicly-fuzzer-packet fuzz/packet.cc ${PICOTLS_OPENSSL_FILES})
ADD_EXECUTABLE(${FUZZER_EXE} ${FUZZER_CC} ${PICOTLS_OPENSSL_FILES} lib/quicly.c)
SET(CMAKE_EXE_LINKER_FLAGS "${CMAKE_C_FLAGS}")
IF (OSS_FUZZ)
# Use https://github.com/google/oss-fuzz compatible options
SET(LIB_FUZZER FuzzingEngine)
SET(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -fno-omit-frame-pointer")
SET(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -fno-omit-frame-pointer")
TARGET_LINK_LIBRARIES(quicly-fuzzer-packet quicly ${OPENSSL_LIBRARIES} ${CMAKE_DL_LIBS})
TARGET_LINK_LIBRARIES(${FUZZER_EXE} quicly ${OPENSSL_LIBRARIES} ${CMAKE_DL_LIBS})
ELSEIF (USE_CLANG_RT)
SET(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -fno-omit-frame-pointer -fsanitize=fuzzer,address,undefined -fsanitize-coverage=edge,indirect-calls")
SET(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -g -fno-omit-frame-pointer -fsanitize=fuzzer,address,undefined -fsanitize-coverage=edge,indirect-calls")
TARGET_LINK_LIBRARIES(quicly-fuzzer-packet quicly ${OPENSSL_LIBRARIES} ${CMAKE_DL_LIBS})
TARGET_LINK_LIBRARIES(${FUZZER_EXE} quicly ${OPENSSL_LIBRARIES} ${CMAKE_DL_LIBS})
ELSE()
SET(LIB_FUZZER "${CMAKE_CURRENT_BINARY_DIR}/libFuzzer.a")
SET(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -fno-omit-frame-pointer -fsanitize=address -fsanitize-address-use-after-scope -fsanitize=fuzzer-no-link")
SET(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -g -fno-omit-frame-pointer -fsanitize=address -fsanitize-address-use-after-scope -fsanitize=fuzzer-no-link")
ADD_CUSTOM_TARGET(libFuzzer ${CMAKE_CURRENT_SOURCE_DIR}/misc/build_libFuzzer.sh WORKING_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR})
ADD_DEPENDENCIES(quicly-fuzzer-packet libFuzzer)
TARGET_LINK_LIBRARIES(quicly-fuzzer-packet quicly ${OPENSSL_LIBRARIES} ${CMAKE_DL_LIBS} ${LIB_FUZZER})
ADD_DEPENDENCIES(${FUZZER_EXE} libFuzzer)
TARGET_LINK_LIBRARIES(${FUZZER_EXE} quicly ${OPENSSL_LIBRARIES} ${CMAKE_DL_LIBS} ${LIB_FUZZER})
ENDIF(OSS_FUZZ)
ENDIF()

IF (BUILD_FUZZER_ADDRESS_TOKEN)
MESSAGE(STATUS "===== Making the adddress token fuzzer")
SET(FUZZER_EXE "quicly-fuzzer-address-token")
SET(FUZZER_CC "fuzz/address_token.cc")

IF(NOT CMAKE_CXX_COMPILER_ID STREQUAL "Clang")
MESSAGE(FATAL_ERROR "The fuzzer needs clang as a compiler")
ENDIF()
ADD_EXECUTABLE(${FUZZER_EXE} ${FUZZER_CC} ${PICOTLS_OPENSSL_FILES})
SET(CMAKE_EXE_LINKER_FLAGS "${CMAKE_C_FLAGS}")
SET(LIB_FUZZER "${CMAKE_CURRENT_BINARY_DIR}/libFuzzer.a")
SET(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -fno-omit-frame-pointer -fsanitize=address -fsanitize-address-use-after-scope -fsanitize=fuzzer-no-link")
SET(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -g -fno-omit-frame-pointer -fsanitize=address -fsanitize-address-use-after-scope -fsanitize=fuzzer-no-link")
ADD_CUSTOM_TARGET(libFuzzer ${CMAKE_CURRENT_SOURCE_DIR}/misc/build_libFuzzer.sh WORKING_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR})
ADD_DEPENDENCIES(${FUZZER_EXE} quicly libFuzzer)
TARGET_LINK_LIBRARIES(${FUZZER_EXE} quicly ${OPENSSL_LIBRARIES} ${CMAKE_DL_LIBS} ${LIB_FUZZER})
ENDIF()

23 changes: 23 additions & 0 deletions fuzz/address_token.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
#include <string.h>
#include <stdio.h>
#include "picotls.h"
#include "picotls/openssl.h"
#include "quicly.h"
#include "quicly/defaults.h"
#include "quicly/streambuf.h"

void __sanitizer_cov_trace_pc(void)
{
}

extern "C" int LLVMFuzzerTestOneInput(const uint8_t *Data, size_t Size)
{

static const uint8_t zero_key[PTLS_MAX_SECRET_SIZE] = {0};
ptls_buffer_t buf;
char b[3];
ptls_aead_context_t *enc = ptls_aead_new(&ptls_openssl_aes128gcm, &ptls_openssl_sha256, 1, zero_key, "");
ptls_buffer_init(&buf, b, 0);

quicly_encrypt_address_token(ptls_openssl_random_bytes, enc, &buf, Size, (quicly_address_token_plaintext_t *)Data);
Copy link
Member

@kazuho kazuho Mar 6, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure if we can or need to fuzz quicly_address_token_plaintext_t, as is not an input from the network. It is a local structure being built with certain restricitons (e.g., quicly_address_t containing a valid sockaddr).

}
1 change: 1 addition & 0 deletions fuzz/packet.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@ extern "C" int LLVMFuzzerTestOneInput(const uint8_t *Data, size_t Size)
if ((ret = quicly_decode_max_streams_frame(&src, end, &frame)) != 0)
return 0;
} break;
case QUICLY_FRAME_TYPE_PADDING:
case QUICLY_FRAME_TYPE_PING:
ret = 0;
break;
Expand Down
5 changes: 4 additions & 1 deletion lib/quicly.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4909,7 +4909,10 @@ void quicly_amend_ptls_context(ptls_context_t *ptls)
int quicly_encrypt_address_token(void (*random_bytes)(void *, size_t), ptls_aead_context_t *aead, ptls_buffer_t *buf,
size_t start_off, const quicly_address_token_plaintext_t *plaintext)
{
int ret;
int ret = 0;

if (start_off < sizeof(quicly_address_token_plaintext_t))
goto Exit;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure if this change is necessary. start_off does not have any relationship with quicly_address_token_plaintext_t.

As stated in the doc-comment of the doc-comment of the function declaration, the role of the function is to append token to the buffer being provided, considering bytes between start_off and buf->off as AAD.

Also, the function is expected to return an error code (0 means success). I'd assume that this is an error case?


/* IV */
if ((ret = ptls_buffer_reserve(buf, aead->algo->iv_size)) != 0)
Expand Down