From b92fcfdc17f3ad794c220a86f4ae6695d0a0fb61 Mon Sep 17 00:00:00 2001 From: David Benjamin Date: Thu, 11 May 2023 18:53:05 -0400 Subject: [PATCH] Cap the input size to the conf fuzzer 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 Commit-Queue: David Benjamin Auto-Submit: David Benjamin --- fuzz/conf.cc | 7 +++++++ include/openssl/x509v3.h | 8 +++++--- 2 files changed, 12 insertions(+), 3 deletions(-) diff --git a/fuzz/conf.cc b/fuzz/conf.cc index eed87f3501..9b810e1029 100644 --- a/fuzz/conf.cc +++ b/fuzz/conf.cc @@ -17,7 +17,14 @@ #include #include +#include + 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_new_mem_buf(buf, len)); bssl::UniquePtr conf(NCONF_new(nullptr)); if (NCONF_load_bio(conf.get(), bio.get(), nullptr)) { diff --git a/include/openssl/x509v3.h b/include/openssl/x509v3.h index d9c862da83..f5ea41354e 100644 --- a/include/openssl/x509v3.h +++ b/include/openssl/x509v3.h @@ -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