Skip to content

Commit

Permalink
Cap the input size to the conf fuzzer
Browse files Browse the repository at this point in the history
Trying to fix all the places where these formats go quadratic isn't a
good use of time. We've already documented that they're not safe for use
with untrusted inputs. Even without such DoS issues, they cannot be
safely used anyway. (E.g. RUSTSEC-2023-0023.)

Just cap the fuzzer input. It'd be nice if we could avoid this more
systematically in the function, but they're not structured to make this
easy to do, and anyone concerned about DoS in this function has worse
problems.

Bug: chromium:1444420, oss-fuzz:56048, 611
Change-Id: I53eeb346f59278ec2db3aac4a92573b927ed8003
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/59785
Reviewed-by: Bob Beck <bbe@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
Auto-Submit: David Benjamin <davidben@google.com>
  • Loading branch information
davidben authored and Boringssl LUCI CQ committed May 12, 2023
1 parent e24491a commit b92fcfd
Show file tree
Hide file tree
Showing 2 changed files with 12 additions and 3 deletions.
7 changes: 7 additions & 0 deletions fuzz/conf.cc
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,14 @@
#include <openssl/x509.h>
#include <openssl/x509v3.h>

#include <algorithm>

extern "C" int LLVMFuzzerTestOneInput(const uint8_t *buf, size_t len) {
// The string-based extensions APIs routinely produce output quadratic in
// their input. Cap the input size to mitigate this. See also
// https://crbug.com/boringssl/611.
len = std::min(len, size_t{8 * 1024});

bssl::UniquePtr<BIO> bio(BIO_new_mem_buf(buf, len));
bssl::UniquePtr<CONF> conf(NCONF_new(nullptr));
if (NCONF_load_bio(conf.get(), bio.get(), nullptr)) {
Expand Down
8 changes: 5 additions & 3 deletions include/openssl/x509v3.h
Original file line number Diff line number Diff line change
Expand Up @@ -549,9 +549,11 @@ OPENSSL_EXPORT void X509V3_conf_free(CONF_VALUE *val);
//
// These functions are not safe to use with untrusted inputs. The string formats
// may implicitly reference context information and, in OpenSSL (though not
// BoringSSL), one even allows reading arbitrary files. They additionally see
// much less testing and review than most of the library and may have bugs
// including memory leaks or crashes.
// BoringSSL), one even allows reading arbitrary files. Many formats can also
// produce far larger outputs than their inputs, so untrusted inputs may lead to
// denial-of-service attacks. Finally, the parsers see much less testing and
// review than most of the library and may have bugs including memory leaks or
// crashes.

// v3_ext_ctx, aka |X509V3_CTX|, contains additional context information for
// constructing extensions. Some string formats reference additional values in
Expand Down

0 comments on commit b92fcfd

Please sign in to comment.