From 6fe4b539c746bff4b6a6e0da2405535bcad11a66 Mon Sep 17 00:00:00 2001 From: Alex Forsythe Date: Fri, 10 Jul 2026 12:11:31 -0500 Subject: [PATCH] fix: Store fixed-size strings inline in `dd_core_config_t` refs: RUM-17284 ## Background In the C++ API, where we accept values like `env`, `version`, `service`, etc. as `std::string_view`, we implicitly copy the application-provided values into `std::string` members. In the C API, we need to hold string values in `dd_core_config_t` initially: when the core is created via `dd_core_create()`, we convert all of our config data to a C++ `CoreConfig` struct ## Problem The C API holds some string values as `char*` - we accept raw string pointers from the application and store them directly. This is fine for static strings, but it represents a use-after-free bug if the application-provided values have limited lifetime: we could crash on `dd_core_create()` if values supplied for client_token, env, version, service, etc., have temporary storage. ## Fix This PR fixes the problem by making `dd_core_config_t` (and its member struct `dd_internal_options_t`) store all string values inline, using fixed-size buffers. These fixed sizes are exposed as public constants: we already took this approach to prevent similar issues in the C logger API. If an application-provided string exceeds the expected maximum size, we truncate it and log a warning. This is a breaking ABI change (and it also renames a public constant in the C API, which technically makes it a breaking API change as well) that only affects the C API. Given the early stage of C API adoption, I think we should be fine to make this change swiftly. After the initial Flutter desktop pass came together, we briefly discussed making more fundamental changes to the way that config objects are allocated and stored in the C API. These changes are not in scope here: this PR is concerned only with preventing this specific class of bugs. --- include-c/datadog/core.h | 28 ++-- include-c/datadog/logging.h | 6 +- src/datadog/c/config_string.hpp | 72 +++++++++++ src/datadog/c/core.cpp | 220 +++++++++++++++++++++++++------- src/datadog/c/logging.cpp | 31 +++-- src/datadog/impl/core/types.hpp | 22 +--- 6 files changed, 293 insertions(+), 86 deletions(-) create mode 100644 src/datadog/c/config_string.hpp diff --git a/include-c/datadog/core.h b/include-c/datadog/core.h index bf41a7f4..597ce89f 100644 --- a/include-c/datadog/core.h +++ b/include-c/datadog/core.h @@ -16,7 +16,15 @@ // These values establish the size of string buffers in the C API; they do not imply // that the Datadog platform imposes any such limits -#define DATADOG_MAX_SERVICE_NAME_LEN 127 +#define DATADOG_MAX_APPLICATION_STORAGE_PATH_LEN 511 +#define DATADOG_MAX_CLIENT_TOKEN_LEN 63 +#define DATADOG_MAX_SERVICE_LEN 127 +#define DATADOG_MAX_ENV_LEN 127 +#define DATADOG_MAX_VERSION_LEN 127 +#define DATADOG_MAX_VARIANT_LEN 127 +#define DATADOG_INTERNAL_MAX_CUSTOM_ENDPOINT_URL_LEN 255 +#define DATADOG_INTERNAL_MAX_SOURCE_LEN 15 +#define DATADOG_INTERNAL_MAX_SDK_VERSION_LEN 31 #ifdef __cplusplus extern "C" { @@ -130,9 +138,9 @@ typedef enum { */ typedef struct dd_internal_options { bool flush_http_requests_on_stop; - const char* custom_endpoint_url; - const char* source; - const char* sdk_version; + char custom_endpoint_url[DATADOG_INTERNAL_MAX_CUSTOM_ENDPOINT_URL_LEN + 1]; + char source[DATADOG_INTERNAL_MAX_SOURCE_LEN + 1]; + char sdk_version[DATADOG_INTERNAL_MAX_SDK_VERSION_LEN + 1]; } dd_internal_options_t; /** @@ -145,13 +153,13 @@ typedef struct dd_core_config { void* diagnostic_handler_userdata; dd_diagnostic_level_t diagnostic_threshold; dd_tracking_consent_t tracking_consent; - char application_storage_path[512]; + char application_storage_path[DATADOG_MAX_APPLICATION_STORAGE_PATH_LEN + 1]; dd_site_t site; - const char* client_token; - const char* service; - const char* env; - const char* application_version; - const char* variant; + char client_token[DATADOG_MAX_CLIENT_TOKEN_LEN + 1]; + char service[DATADOG_MAX_SERVICE_LEN + 1]; + char env[DATADOG_MAX_ENV_LEN + 1]; + char application_version[DATADOG_MAX_VERSION_LEN + 1]; + char variant[DATADOG_MAX_VARIANT_LEN + 1]; dd_batch_size_t batch_size; dd_upload_frequency_t upload_frequency; dd_batch_processing_level_t batch_processing_level; diff --git a/include-c/datadog/logging.h b/include-c/datadog/logging.h index 3b2ae055..084421ba 100644 --- a/include-c/datadog/logging.h +++ b/include-c/datadog/logging.h @@ -43,7 +43,7 @@ typedef enum { typedef struct dd_logger_config { uint32_t version; float remote_sample_rate; - char service[DATADOG_MAX_SERVICE_NAME_LEN + 1]; + char service[DATADOG_MAX_SERVICE_LEN + 1]; char name[DATADOG_MAX_LOGGER_NAME_LEN + 1]; dd_log_level_t remote_log_threshold; size_t initial_attribute_capacity; @@ -69,8 +69,8 @@ DATADOG_API void dd_logger_config_set_remote_sample_rate( * logger will use the service name configured globally via dd_core_config_t. * * Config stores a copy of provided string value. If the given string value exceeds - * DATADOG_MAX_SERVICE_NAME_LEN, it will be truncated to that length. Both NULL and "" - * will be interepreted as no value, causing the logger to use the default service name. + * DATADOG_MAX_SERVICE_LEN, it will be truncated to that length. Both NULL and "" will + * be interepreted as no value, causing the logger to use the default service name. */ DATADOG_API void dd_logger_config_set_service( dd_logger_config_t* config, const char* value diff --git a/src/datadog/c/config_string.hpp b/src/datadog/c/config_string.hpp new file mode 100644 index 00000000..4c010783 --- /dev/null +++ b/src/datadog/c/config_string.hpp @@ -0,0 +1,72 @@ +// Unless explicitly stated otherwise all files in this repository are licensed +// under the Apache License Version 2.0. +// +// This product includes software developed at Datadog (https://www.datadoghq.com/). +// Copyright 2025-Present Datadog, Inc. + +#pragma once + +#include +#include +#include + +#include "datadog/impl/core/util/diagnostics.hpp" + +// Expands a macro constant to a string literal, e.g. DATADOG_CSTR(63) -> "63". +// Used to embed DATADOG_MAX_* limit values in static diagnostic message strings +// without any runtime formatting. +#define DATADOG_CSTR_(x) #x +#define DATADOG_CSTR(x) DATADOG_CSTR_(x) + +/** + * Copies `src` into the fixed-size buffer `dest[0..dest_size)`, truncating if + * the source string is too long. If truncation occurs, emits `truncation_msg` + * at warning level via `logger`. Both `src` and `truncation_msg` may be null: + * a null or empty `src` writes an empty string; a null `truncation_msg` skips + * the diagnostic (intended for callers with no diagnostic handler available). + */ +static inline void assign_string_truncate( + char* dest, + size_t dest_size, + const char* src, + const datadog::impl::DiagnosticLogger& logger, + const char* truncation_msg +) { + if (!src || src[0] == '\0') { + dest[0] = '\0'; + return; + } + // NOLINTNEXTLINE(cppcoreguidelines-pro-type-vararg) + int written = std::snprintf(dest, dest_size, "%s", src); + if (truncation_msg && written >= 0 && static_cast(written) >= dest_size) { + logger.Warning(truncation_msg); + } +} + +/** + * Copies `src` into the fixed-size buffer `dest[0..dest_size)`, refusing if + * the source string would overflow the buffer. A null or empty `src` writes an + * empty string. If the string is too long, emits `overflow_msg` at error level + * via `logger` and leaves `dest` unchanged; pass a null `overflow_msg` to + * suppress the diagnostic. + */ +static inline void assign_string_strict( + char* dest, + size_t dest_size, + const char* src, + const datadog::impl::DiagnosticLogger& logger, + const char* overflow_msg +) { + if (!src || src[0] == '\0') { + dest[0] = '\0'; + return; + } + if (!std::memchr(src, '\0', dest_size)) { + if (overflow_msg) { + logger.Error(overflow_msg); + } + return; + } + // NOLINTNEXTLINE(cppcoreguidelines-pro-type-vararg) + std::snprintf(dest, dest_size, "%s", src); +} diff --git a/src/datadog/c/core.cpp b/src/datadog/c/core.cpp index 50453b3a..29f2b13a 100644 --- a/src/datadog/c/core.cpp +++ b/src/datadog/c/core.cpp @@ -9,12 +9,14 @@ #include #include +#include "datadog/c/config_string.hpp" #include "datadog/c/core_glue.hpp" #include "datadog/impl/core/attribute/types.hpp" #include "datadog/impl/core/core.hpp" #include "datadog/impl/core/types.hpp" // NOLINTBEGIN(cppcoreguidelines-owning-memory) +// NOLINTBEGIN(cppcoreguidelines-pro-bounds-array-to-pointer-decay) static const uint32_t CORE_CONFIG_VERSION = 1; @@ -24,21 +26,21 @@ static const dd_core_config_t DEFAULT_CORE_CONFIG = { nullptr, // diagnostic_handler_userdata DD_DIAGNOSTIC_LEVEL_WARNING, // diagnostic_threshold DD_TRACKING_CONSENT_PENDING, // tracking_consent - {0}, // application_storage_path + "", // application_storage_path DD_SITE_US1, // site - nullptr, // client_token - nullptr, // service - nullptr, // env - nullptr, // application_version - nullptr, // variant + "", // client_token + "", // service + "", // env + "", // application_version + "", // variant DD_BATCH_SIZE_MEDIUM, // batch_size DD_UPLOAD_FREQUENCY_AVERAGE, // upload_frequency DD_BATCH_PROCESSING_LEVEL_MEDIUM, // batch_processing_level { - false, // internal_options.flush_http_requests_on_stop - nullptr, // internal_options.custom_endpoint_url - nullptr, // internal_options.source - nullptr // internal_options.sdk_version + false, // internal_options.flush_http_requests_on_stop + "", // internal_options.custom_endpoint_url + "", // internal_options.source + "" // internal_options.sdk_version } }; @@ -61,9 +63,37 @@ void dd_core_config_init( return; } *config = DEFAULT_CORE_CONFIG; - config->client_token = client_token; - config->service = service; - config->env = env; + auto logger = datadog::impl::DiagnosticLogger::FromC( + config->diagnostic_handler, + config->diagnostic_handler_userdata, + config->diagnostic_threshold + ); + assign_string_truncate( + config->client_token, + sizeof(config->client_token), + client_token, + logger, + "client_token value passed to dd_core_config_init exceeds " + "DATADOG_MAX_CLIENT_TOKEN_LEN (" DATADOG_CSTR( + DATADOG_MAX_CLIENT_TOKEN_LEN + ) ") and will be truncated" + ); + assign_string_truncate( + config->service, + sizeof(config->service), + service, + logger, + "service value passed to dd_core_config_init exceeds DATADOG_MAX_SERVICE_LEN " + "(" DATADOG_CSTR(DATADOG_MAX_SERVICE_LEN) ") and will be truncated" + ); + assign_string_truncate( + config->env, + sizeof(config->env), + env, + logger, + "env value passed to dd_core_config_init exceeds DATADOG_MAX_ENV_LEN " + "(" DATADOG_CSTR(DATADOG_MAX_ENV_LEN) ") and will be truncated" + ); } void dd_core_config_set_diagnostic_handler( @@ -99,27 +129,21 @@ void dd_core_config_set_application_storage_path( if (!config || !value) { return; } - const size_t capacity = std::size(config->application_storage_path); - const std::size_t max_len = capacity - 1; - const void* p_null{ - std::memchr(value, '\0', max_len + 1) - }; // NOLINT(cppcoreguidelines-init-variables) - if (!p_null) { - auto diagnostic_logger = datadog::impl::DiagnosticLogger::FromC( - config->diagnostic_handler, - config->diagnostic_handler_userdata, - config->diagnostic_threshold - ); - diagnostic_logger.Error( - "Unable to accept value passed to dd_core_config_set_application_storage_path: " - "length limit exceeded" - ); - return; - } - const size_t len = static_cast(p_null) - value; - std::memcpy(static_cast(config->application_storage_path), value, len); - // NOLINTNEXTLINE(cppcoreguidelines-pro-bounds-constant-array-index) - config->application_storage_path[len] = '\0'; + auto logger = datadog::impl::DiagnosticLogger::FromC( + config->diagnostic_handler, + config->diagnostic_handler_userdata, + config->diagnostic_threshold + ); + assign_string_strict( + config->application_storage_path, + sizeof(config->application_storage_path), + value, + logger, + "application_storage_path value exceeds " + "DATADOG_MAX_APPLICATION_STORAGE_PATH_LEN (" DATADOG_CSTR( + DATADOG_MAX_APPLICATION_STORAGE_PATH_LEN + ) ") and will not be accepted" + ); } void dd_core_config_set_site(dd_core_config_t* config, dd_site_t value) { @@ -133,35 +157,100 @@ void dd_core_config_set_client_token(dd_core_config_t* config, const char* value if (!config) { return; } - config->client_token = value; + auto logger = datadog::impl::DiagnosticLogger::FromC( + config->diagnostic_handler, + config->diagnostic_handler_userdata, + config->diagnostic_threshold + ); + assign_string_truncate( + config->client_token, + sizeof(config->client_token), + value, + logger, + "client_token value exceeds DATADOG_MAX_CLIENT_TOKEN_LEN (" DATADOG_CSTR( + DATADOG_MAX_CLIENT_TOKEN_LEN + ) ") and will be truncated" + ); } void dd_core_config_set_service(dd_core_config_t* config, const char* value) { if (!config) { return; } - config->service = value; + auto logger = datadog::impl::DiagnosticLogger::FromC( + config->diagnostic_handler, + config->diagnostic_handler_userdata, + config->diagnostic_threshold + ); + assign_string_truncate( + config->service, + sizeof(config->service), + value, + logger, + "service value exceeds DATADOG_MAX_SERVICE_LEN (" DATADOG_CSTR( + DATADOG_MAX_SERVICE_LEN + ) ") and will be truncated" + ); } void dd_core_config_set_env(dd_core_config_t* config, const char* value) { if (!config) { return; } - config->env = value; + auto logger = datadog::impl::DiagnosticLogger::FromC( + config->diagnostic_handler, + config->diagnostic_handler_userdata, + config->diagnostic_threshold + ); + assign_string_truncate( + config->env, + sizeof(config->env), + value, + logger, + "env value exceeds DATADOG_MAX_ENV_LEN (" DATADOG_CSTR( + DATADOG_MAX_ENV_LEN + ) ") and will be truncated" + ); } void dd_core_config_set_version(dd_core_config_t* config, const char* value) { if (!config) { return; } - config->application_version = value; + auto logger = datadog::impl::DiagnosticLogger::FromC( + config->diagnostic_handler, + config->diagnostic_handler_userdata, + config->diagnostic_threshold + ); + assign_string_truncate( + config->application_version, + sizeof(config->application_version), + value, + logger, + "application version value exceeds DATADOG_MAX_VERSION_LEN (" DATADOG_CSTR( + DATADOG_MAX_VERSION_LEN + ) ") and will be truncated" + ); } void dd_core_config_set_variant(dd_core_config_t* config, const char* value) { if (!config) { return; } - config->variant = value; + auto logger = datadog::impl::DiagnosticLogger::FromC( + config->diagnostic_handler, + config->diagnostic_handler_userdata, + config->diagnostic_threshold + ); + assign_string_truncate( + config->variant, + sizeof(config->variant), + value, + logger, + "variant value exceeds DATADOG_MAX_VARIANT_LEN (" DATADOG_CSTR( + DATADOG_MAX_VARIANT_LEN + ) ") and will be truncated" + ); } void dd_core_config_set_batch_size(dd_core_config_t* config, dd_batch_size_t value) { @@ -202,14 +291,41 @@ void dd_core_config_internal_set_custom_endpoint_url( if (!config) { return; } - config->internal_options.custom_endpoint_url = value; + auto logger = datadog::impl::DiagnosticLogger::FromC( + config->diagnostic_handler, + config->diagnostic_handler_userdata, + config->diagnostic_threshold + ); + assign_string_truncate( + config->internal_options.custom_endpoint_url, + sizeof(config->internal_options.custom_endpoint_url), + value, + logger, + "custom endpoint URL value exceeds DATADOG_INTERNAL_MAX_CUSTOM_ENDPOINT_URL_LEN " + "(" DATADOG_CSTR( + DATADOG_INTERNAL_MAX_CUSTOM_ENDPOINT_URL_LEN + ) ") and will be truncated" + ); } void dd_core_config_internal_set_source(dd_core_config_t* config, const char* value) { if (!config) { return; } - config->internal_options.source = value; + auto logger = datadog::impl::DiagnosticLogger::FromC( + config->diagnostic_handler, + config->diagnostic_handler_userdata, + config->diagnostic_threshold + ); + assign_string_truncate( + config->internal_options.source, + sizeof(config->internal_options.source), + value, + logger, + "source value exceeds DATADOG_INTERNAL_MAX_SOURCE_LEN (" DATADOG_CSTR( + DATADOG_INTERNAL_MAX_SOURCE_LEN + ) ") and will be truncated" + ); } void dd_core_config_internal_set_sdk_version( @@ -218,7 +334,20 @@ void dd_core_config_internal_set_sdk_version( if (!config) { return; } - config->internal_options.sdk_version = value; + auto logger = datadog::impl::DiagnosticLogger::FromC( + config->diagnostic_handler, + config->diagnostic_handler_userdata, + config->diagnostic_threshold + ); + assign_string_truncate( + config->internal_options.sdk_version, + sizeof(config->internal_options.sdk_version), + value, + logger, + "SDK version value exceeds DATADOG_INTERNAL_MAX_SDK_VERSION_LEN (" DATADOG_CSTR( + DATADOG_INTERNAL_MAX_SDK_VERSION_LEN + ) ") and will be truncated" + ); } dd_core_t* dd_core_create( @@ -245,21 +374,21 @@ dd_core_t* dd_core_create( ); // Likewise, if the config is missing any required values, reject it - if (!config->client_token || !config->client_token[0]) { + if (!config->client_token[0]) { diagnostic_logger.Error( "SDK initialization failed: application must supply a non-empty 'client_token' " "value in dd_core_config_t" ); return nullptr; } - if (!config->service || !config->service[0]) { + if (!config->service[0]) { diagnostic_logger.Error( "SDK initialization failed: application must supply a non-empty 'service' " "value in dd_core_config_t" ); return nullptr; } - if (!config->env || !config->env[0]) { + if (!config->env[0]) { diagnostic_logger.Error( "SDK initialization failed: application must supply a non-empty 'env' value in " "dd_core_config_t" @@ -387,4 +516,5 @@ void dd_core_stop(dd_core_t* core) { } } +// NOLINTEND(cppcoreguidelines-pro-bounds-array-to-pointer-decay) // NOLINTEND(cppcoreguidelines-owning-memory) diff --git a/src/datadog/c/logging.cpp b/src/datadog/c/logging.cpp index 40c53d7e..e227ef72 100644 --- a/src/datadog/c/logging.cpp +++ b/src/datadog/c/logging.cpp @@ -12,6 +12,7 @@ #include "datadog/core.h" +#include "datadog/c/config_string.hpp" #include "datadog/c/core_glue.hpp" #include "datadog/c/logging_glue.hpp" #include "datadog/impl/core/attribute/types.hpp" @@ -56,23 +57,29 @@ void dd_logger_config_set_remote_sample_rate(dd_logger_config_t* config, float v } void dd_logger_config_set_service(dd_logger_config_t* config, const char* value) { - if (config) { - if (value) { - std::snprintf(config->service, sizeof(config->service), "%s", value); - } else { - config->service[0] = '\0'; - } + if (!config) { + return; } + assign_string_truncate( + config->service, + sizeof(config->service), + value, + datadog::impl::DiagnosticLogger{}, + nullptr + ); } void dd_logger_config_set_name(dd_logger_config_t* config, const char* value) { - if (config) { - if (value) { - std::snprintf(config->name, sizeof(config->name), "%s", value); - } else { - config->name[0] = '\0'; - } + if (!config) { + return; } + assign_string_truncate( + config->name, + sizeof(config->name), + value, + datadog::impl::DiagnosticLogger{}, + nullptr + ); } void dd_logger_config_set_remote_log_threshold( diff --git a/src/datadog/impl/core/types.hpp b/src/datadog/impl/core/types.hpp index ca212937..3616c1fc 100644 --- a/src/datadog/impl/core/types.hpp +++ b/src/datadog/impl/core/types.hpp @@ -230,26 +230,17 @@ inline const char* BatchProcessingLevel_ToString(BatchProcessingLevel value) { } inline CoreConfig CoreConfig_FromC(const dd_core_config_t& config) { - // Convert all of the C struct's string values to std::string_view safely - std::string_view client_token = - config.client_token != nullptr ? config.client_token : ""; - std::string_view service = config.service != nullptr ? config.service : ""; - std::string_view env = config.env != nullptr ? config.env : ""; - std::string_view application_version = - config.application_version ? config.application_version : ""; - std::string_view variant = config.variant ? config.variant : ""; - // Initialize a C++ config struct from our input values auto cpp_config = - CoreConfig(client_token, service, env) + CoreConfig(config.client_token, config.service, config.env) .SetDiagnosticHandler(DiagnosticHandler_FromC( config.diagnostic_handler, config.diagnostic_handler_userdata )) .SetDiagnosticThreshold(DiagnosticLevel_FromC(config.diagnostic_threshold)) .SetApplicationStoragePath(config.application_storage_path) .SetSite(Site_FromC(config.site)) - .SetVersion(application_version) - .SetVariant(variant) + .SetVersion(config.application_version) + .SetVariant(config.variant) .SetBatchSize(BatchSize_FromC(config.batch_size)) .SetUploadFrequency(UploadFrequency_FromC(config.upload_frequency)) .SetBatchProcessingLevel( @@ -260,14 +251,13 @@ inline CoreConfig CoreConfig_FromC(const dd_core_config_t& config) { if (config.internal_options.flush_http_requests_on_stop) { cpp_config.Internal_FlushHttpRequestsOnStop(); } - if (config.internal_options.custom_endpoint_url && - config.internal_options.custom_endpoint_url[0]) { + if (config.internal_options.custom_endpoint_url[0]) { cpp_config.Internal_UseCustomEndpoint(config.internal_options.custom_endpoint_url); } - if (config.internal_options.source && config.internal_options.source[0]) { + if (config.internal_options.source[0]) { cpp_config.Internal_SetSource(config.internal_options.source); } - if (config.internal_options.sdk_version && config.internal_options.sdk_version[0]) { + if (config.internal_options.sdk_version[0]) { cpp_config.Internal_SetSdkVersion(config.internal_options.sdk_version); }