From af1befed6fb4ca0c4118d630034073d05c9ccfa5 Mon Sep 17 00:00:00 2001 From: Constantinos Date: Mon, 3 Aug 2026 03:12:59 +1000 Subject: [PATCH] Validate .nam model config fields instead of unchecked operator[] .nam files are untrusted input: hosts download and open them on behalf of users. The config parsers read required fields with `config["key"]`, which is nlohmann's const operator[]. That does JSON_ASSERT(key exists) then dereferences. JSON_ASSERT is plain assert (json.hpp:2571), so under NDEBUG -- i.e. any release plugin build -- a model file missing a required key dereferences a past-the-end iterator instead of throwing. Add nam::util helpers (NAM/json_util.h) that look fields up, report the offending key and its enclosing context, and throw std::runtime_error -- matching the "bad model file" convention already used throughout the loader. Route the required-field reads in the envelope (get_dsp), the WaveNet config parser, the Linear parser, and the activation config through them. Also validate values that previously flowed unchecked into allocations and arithmetic: - Dimensions (channels, input_size, condition_size, receptive_field, kernel sizes) are bounded to [1, 65536]. Previously a negative value became a huge size_t in resize() and any int was accepted. - groups fields must be >= 1. `groups_input: 0` reached `x % groups` and divided by zero. - Array lengths (layers, dilations, kernel_sizes) are bounded, so a small file cannot request millions of Layer objects. - Non-integral numbers are rejected rather than silently truncated. Fields that were optional stay optional, and no accepted value changes meaning, so any model that loads today still loads identically. Only inputs that were previously undefined behaviour now produce an error. Adds tools/test/test_model_validation.cpp covering each rejection plus positive tests that a well-formed WaveNet and Linear model still load. This does not fix the truncated-weights over-read (set_weights_ walks the weight vector with unchecked *(it++)); that needs a bounds-checked reader and is left for a follow-up. --- NAM/activations.cpp | 14 +- NAM/get_dsp.cpp | 13 +- NAM/get_dsp.h | 25 ++ NAM/json_util.cpp | 110 ++++++++ NAM/json_util.h | 118 +++++++++ NAM/linear.cpp | 14 +- NAM/wavenet/model.cpp | 119 +++++---- tools/run_tests.cpp | 24 ++ tools/test/test_model_validation.cpp | 379 +++++++++++++++++++++++++++ 9 files changed, 758 insertions(+), 58 deletions(-) create mode 100644 NAM/json_util.cpp create mode 100644 NAM/json_util.h create mode 100644 tools/test/test_model_validation.cpp diff --git a/NAM/activations.cpp b/NAM/activations.cpp index 3e0bc944..6d7b8b4f 100644 --- a/NAM/activations.cpp +++ b/NAM/activations.cpp @@ -91,7 +91,19 @@ nam::activations::ActivationConfig nam::activations::ActivationConfig::from_json // If it's an object, parse type and parameters if (j.is_object()) { - std::string type_str = j["type"].get(); + // `j` is a const reference, so `j["type"]` would resolve to nlohmann's const + // `operator[]`, which asserts (UB under -DNDEBUG) if "type" is missing. Look it up + // through `find()` instead so a missing field throws instead of crashing. + const auto type_it = j.find("type"); + if (type_it == j.end()) + { + throw std::runtime_error("Activation config: missing required field 'type'"); + } + if (!type_it->is_string()) + { + throw std::runtime_error("Activation config: field 'type' must be a string"); + } + std::string type_str = type_it->get(); auto it = type_map.find(type_str); if (it == type_map.end()) { diff --git a/NAM/get_dsp.cpp b/NAM/get_dsp.cpp index 64393646..9bb7ee1b 100644 --- a/NAM/get_dsp.cpp +++ b/NAM/get_dsp.cpp @@ -9,6 +9,7 @@ #include "registry.h" #include "json.hpp" #include "get_dsp.h" +#include "json_util.h" #include "model_config.h" namespace nam @@ -141,13 +142,17 @@ std::vector GetWeights(nlohmann::json const& j) void populate_dsp_data(const nlohmann::json& config, dspData& returnedConfig) { - verify_config_version(config["version"].get()); + static constexpr const char* kContext = "Model file"; - nlohmann::json config_json = config["config"]; + const std::string version = nam::util::RequireValue(config, "version", kContext); + verify_config_version(version); + + const nlohmann::json& config_json = nam::util::RequireField(config, "config", kContext); + const std::string architecture = nam::util::RequireValue(config, "architecture", kContext); std::vector weights = GetWeights(config); - returnedConfig.version = config["version"].get(); - returnedConfig.architecture = config["architecture"].get(); + returnedConfig.version = version; + returnedConfig.architecture = architecture; returnedConfig.config = config_json; returnedConfig.metadata = config.value("metadata", nlohmann::json()); returnedConfig.weights = weights; diff --git a/NAM/get_dsp.h b/NAM/get_dsp.h index c9e7941b..46511c28 100644 --- a/NAM/get_dsp.h +++ b/NAM/get_dsp.h @@ -77,16 +77,32 @@ struct DspLoadOptions std::optional prewarm = std::nullopt; }; +// A note on exceptions: `.nam` files are untrusted input (they're downloaded from the +// internet), and the functions below are the load path for them. Malformed input is reported +// by throwing--most commonly a `std::runtime_error` raised by a `NAM/json_util.h` validation +// helper, but some code paths still surface a raw `nlohmann::json` parse/type/out-of-range +// exception (`nlohmann::detail::exception`, which derives from `std::exception` but NOT from +// `std::runtime_error`). Callers should therefore `catch (const std::exception&)`, not +// `catch (const std::runtime_error&)`--the latter will miss some malformed-input cases and +// the exception will propagate past the handler (`std::terminate()` if nothing else catches +// it). + /// \brief Get NAM from a .nam file at the provided location /// \param config_filename Path to the .nam model file /// \param options Loading options /// \return Unique pointer to a DSP object +/// \throws std::exception (typically std::runtime_error, but see the note above) if the file +/// doesn't exist or the model file is malformed (missing/invalid required fields, +/// unsupported version, etc.) std::unique_ptr get_dsp(const std::filesystem::path config_filename, DspLoadOptions options = DspLoadOptions()); /// \brief Get NAM from a provided configuration struct /// \param conf DSP data structure containing model configuration and weights /// \param options Loading options /// \return Unique pointer to a DSP object +/// \throws std::exception (typically std::runtime_error, but see the note above) if the model +/// configuration is malformed (missing/invalid required fields, unsupported version, +/// etc.) std::unique_ptr get_dsp(dspData& conf, DspLoadOptions options = DspLoadOptions()); /// \brief Get NAM from a .nam file and store its configuration @@ -96,6 +112,9 @@ std::unique_ptr get_dsp(dspData& conf, DspLoadOptions options = DspLoadOpti /// \param returnedConfig Output parameter that will be filled with the model data /// \param options Loading options /// \return Unique pointer to a DSP object +/// \throws std::exception (typically std::runtime_error, but see the note above) if the file +/// doesn't exist or the model file is malformed (missing/invalid required fields, +/// unsupported version, etc.) std::unique_ptr get_dsp(const std::filesystem::path config_filename, dspData& returnedConfig, DspLoadOptions options = DspLoadOptions()); @@ -104,6 +123,9 @@ std::unique_ptr get_dsp(const std::filesystem::path config_filename, dspDat /// \param returnedConfig Output parameter that will be filled with the model data /// \param options Loading options /// \return Unique pointer to a DSP object +/// \throws std::exception (typically std::runtime_error, but see the note above) if the model +/// configuration is malformed (missing/invalid required fields, unsupported version, +/// etc.) std::unique_ptr get_dsp(const nlohmann::json& config, dspData& returnedConfig, DspLoadOptions options = DspLoadOptions()); @@ -111,6 +133,9 @@ std::unique_ptr get_dsp(const nlohmann::json& config, dspData& returnedConf /// \param config JSON configuration object /// \param options Loading options /// \return Unique pointer to a DSP object +/// \throws std::exception (typically std::runtime_error, but see the note above) if the model +/// configuration is malformed (missing/invalid required fields, unsupported version, +/// etc.) std::unique_ptr get_dsp(const nlohmann::json& config, DspLoadOptions options = DspLoadOptions()); /// \brief Get sample rate from a .nam file diff --git a/NAM/json_util.cpp b/NAM/json_util.cpp new file mode 100644 index 00000000..5bc675c9 --- /dev/null +++ b/NAM/json_util.cpp @@ -0,0 +1,110 @@ +#include "json_util.h" + +#include +#include + +namespace nam +{ +namespace util +{ +const nlohmann::json& RequireField(const nlohmann::json& j, const char* key, const char* context) +{ + if (!j.is_object()) + { + throw std::runtime_error(std::string(context) + ": expected a JSON object containing '" + key + "'"); + } + const auto it = j.find(key); + if (it == j.end()) + { + throw std::runtime_error(std::string(context) + ": missing required field '" + key + "'"); + } + return *it; +} + +namespace +{ +// nlohmann::json's `.get()` silently truncates a stored JSON float (e.g. `4.9` -> `4`) +// rather than rejecting it, which would let a hostile file smuggle a non-integral value +// through a dimension check. Require the underlying value to actually be a JSON integer. +int RequireIntegralValue(const nlohmann::json& value, const char* key, const char* context) +{ + if (!value.is_number_integer()) + { + throw std::runtime_error(std::string(context) + ": field '" + key + "' must be an integer"); + } + return value.get(); +} +} // namespace + +int RequireDimension(const nlohmann::json& j, const char* key, const char* context, int maxValue) +{ + const nlohmann::json& value_json = RequireField(j, key, context); + const int value = RequireIntegralValue(value_json, key, context); + if (value < 1 || value > maxValue) + { + std::stringstream ss; + ss << context << ": field '" << key << "' (" << value << ") must be between 1 and " << maxValue; + throw std::runtime_error(ss.str()); + } + return value; +} + +int OptionalDimension(const nlohmann::json& j, const char* key, const char* context, int defaultValue, int maxValue) +{ + if (!j.is_object()) + { + throw std::runtime_error(std::string(context) + ": expected a JSON object containing '" + key + "'"); + } + const auto it = j.find(key); + if (it == j.end() || it->is_null()) + { + return defaultValue; + } + const int value = RequireIntegralValue(*it, key, context); + if (value < 1 || value > maxValue) + { + std::stringstream ss; + ss << context << ": field '" << key << "' (" << value << ") must be between 1 and " << maxValue; + throw std::runtime_error(ss.str()); + } + return value; +} + +std::vector RequireIntArray(const nlohmann::json& j, const char* key, const char* context, int minValue, + int maxValue, bool allowEmpty, int maxLength) +{ + const nlohmann::json& arr = RequireField(j, key, context); + if (!arr.is_array()) + { + throw std::runtime_error(std::string(context) + ": field '" + key + "' must be an array"); + } + if (!allowEmpty && arr.empty()) + { + throw std::runtime_error(std::string(context) + ": field '" + key + "' must not be empty"); + } + if (arr.size() > static_cast(maxLength)) + { + std::stringstream ss; + ss << context << ": field '" << key << "' has " << arr.size() << " elements, which exceeds the limit of " + << maxLength; + throw std::runtime_error(ss.str()); + } + + std::vector values; + values.reserve(arr.size()); + for (const auto& element : arr) + { + const int value = RequireIntegralValue(element, key, context); + if (value < minValue || value > maxValue) + { + std::stringstream ss; + ss << context << ": field '" << key << "' contains " << value << ", which must be between " << minValue << " and " + << maxValue; + throw std::runtime_error(ss.str()); + } + values.push_back(value); + } + return values; +} +}; // namespace util +}; // namespace nam diff --git a/NAM/json_util.h b/NAM/json_util.h new file mode 100644 index 00000000..ff6def69 --- /dev/null +++ b/NAM/json_util.h @@ -0,0 +1,118 @@ +#pragma once + +// Helpers for safely reading required fields out of an untrusted .nam model-file JSON +// document. +// +// nlohmann::json's `operator[]` on a `const` object asserts (via `JSON_ASSERT`, which is +// plain `assert()`) that the requested key exists before dereferencing it. Under +// `-DNDEBUG` (a typical plugin host Release build) a missing key is therefore undefined +// behavior rather than a thrown exception. `.nam` files are downloaded from the internet, +// so any field the loader treats as required must be looked up through the helpers below +// instead of `operator[]`. +// +// Deliberately kept free of Eigen so that consumers of this header don't need to pull it +// in (see NAM/util.h, which does include Eigen). + +#include +#include +#include + +#include "json.hpp" + +namespace nam +{ +namespace util +{ +/// \brief Memory-safety bound for integer dimensions read from a model file (e.g. channel +/// counts, layer counts). This is not a claim about the size of legitimate models--it only +/// exists to stop a hostile value from driving an unbounded allocation. +constexpr int kMaxModelDimension = 1 << 16; + +/// \brief Memory-safety bound for the LENGTH of arrays read from a model file (e.g. the +/// "layers" array, or a per-layer "dilations"/"kernel_sizes" array). Each element of such an +/// array typically drives construction of a heap-allocated object (a `Layer`, a `LayerArray`), +/// so an unbounded array length lets a tiny, highly-compressible file request an unbounded +/// number of allocations. This is not a claim about the size of legitimate models--real +/// WaveNets have tens of layers, not thousands--it only exists to bound the cost of parsing a +/// hostile file. +constexpr int kMaxModelArrayLength = 4096; + +/// \brief Look up a required key in a JSON object, throwing if it is absent. +/// \param j The JSON object to search +/// \param key The required key +/// \param context Human-readable description of the enclosing object, used in the error +/// message (e.g. "WaveNet layer array 2") +/// \return Reference to the value at `key` +/// \throws std::runtime_error If `j` is not an object or `key` is not present +const nlohmann::json& RequireField(const nlohmann::json& j, const char* key, const char* context); + +/// \brief Look up a required key and convert its value to `T`, throwing if the key is +/// absent or the value can't be converted. +/// \param j The JSON object to search +/// \param key The required key +/// \param context Human-readable description of the enclosing object, used in the error +/// message +/// \return The value at `key`, converted to `T` +/// \throws std::runtime_error If `j` is not an object, `key` is not present, or the value +/// can't be converted to `T` +template +T RequireValue(const nlohmann::json& j, const char* key, const char* context) +{ + const nlohmann::json& value = RequireField(j, key, context); + try + { + return value.get(); + } + catch (const nlohmann::json::exception& e) + { + throw std::runtime_error(std::string(context) + ": field '" + key + "' has the wrong type (" + e.what() + ")"); + } +} + +/// \brief Look up a required integer dimension (e.g. a channel count), enforcing that it +/// falls within `[1, maxValue]`. +/// \param j The JSON object to search +/// \param key The required key +/// \param context Human-readable description of the enclosing object, used in the error +/// message +/// \param maxValue Inclusive upper bound on the returned value +/// \return The validated dimension +/// \throws std::runtime_error If the key is absent, isn't an integer (a JSON float such as +/// `4.9` is rejected rather than silently truncated), or is outside `[1, maxValue]` +int RequireDimension(const nlohmann::json& j, const char* key, const char* context, int maxValue = kMaxModelDimension); + +/// \brief Look up an OPTIONAL integer dimension, enforcing that it falls within +/// `[1, maxValue]` when present. Use this for fields that default to a fixed value when +/// absent (e.g. `in_channels` defaulting to 1)--absence is fine, but a present-and-hostile +/// value (e.g. 0, negative, non-integral, or absurdly large) is not. +/// \param j The JSON object to search +/// \param key The optional key +/// \param context Human-readable description of the enclosing object, used in the error +/// message +/// \param defaultValue Value to return if `key` is absent +/// \param maxValue Inclusive upper bound on the returned value +/// \return `defaultValue` if `key` is absent, otherwise the validated value +/// \throws std::runtime_error If `j` is not an object, or `key` is present but isn't an +/// integer or is outside `[1, maxValue]` +int OptionalDimension(const nlohmann::json& j, const char* key, const char* context, int defaultValue, + int maxValue = kMaxModelDimension); + +/// \brief Look up a required array of integers, enforcing that every element falls within +/// `[minValue, maxValue]` and that the array itself isn't longer than `maxLength`. +/// \param j The JSON object to search +/// \param key The required key +/// \param context Human-readable description of the enclosing object, used in the error +/// message +/// \param minValue Inclusive lower bound on every element +/// \param maxValue Inclusive upper bound on every element +/// \param allowEmpty Whether an empty array is acceptable +/// \param maxLength Inclusive upper bound on the array's length (see `kMaxModelArrayLength`) +/// \return The validated array +/// \throws std::runtime_error If the key is absent, isn't an array of integers (a JSON float +/// element such as `4.9` is rejected rather than silently truncated), is empty when +/// `allowEmpty` is false, is longer than `maxLength`, or contains an out-of-range +/// element +std::vector RequireIntArray(const nlohmann::json& j, const char* key, const char* context, int minValue, + int maxValue, bool allowEmpty = false, int maxLength = kMaxModelArrayLength); +}; // namespace util +}; // namespace nam diff --git a/NAM/linear.cpp b/NAM/linear.cpp index 186ef8ae..fc54c8a2 100644 --- a/NAM/linear.cpp +++ b/NAM/linear.cpp @@ -5,6 +5,7 @@ #include #include +#include "json_util.h" #include "registry.h" #include @@ -305,12 +306,15 @@ std::string nam::linear::implementation_to_string(const LinearImplementation imp nam::linear::LinearConfig nam::linear::parse_config_json(const nlohmann::json& config) { + static constexpr const char* kContext = "Linear config"; + LinearConfig c; - c.receptive_field = config["receptive_field"]; - c.bias = config["bias"]; - // Default to 1 channel in/out for backward compatibility - c.in_channels = config.value("in_channels", 1); - c.out_channels = config.value("out_channels", 1); + c.receptive_field = nam::util::RequireDimension(config, "receptive_field", kContext); + c.bias = nam::util::RequireValue(config, "bias", kContext); + // Default to 1 channel in/out for backward compatibility, but a present-and-hostile value + // feeds a buffer resize()--validate it when present. + c.in_channels = nam::util::OptionalDimension(config, "in_channels", kContext, 1); + c.out_channels = nam::util::OptionalDimension(config, "out_channels", kContext, 1); c.implementation = parse_implementation(config.value("implementation", "auto")); return c; } diff --git a/NAM/wavenet/model.cpp b/NAM/wavenet/model.cpp index 7fd9ed84..03d01d50 100644 --- a/NAM/wavenet/model.cpp +++ b/NAM/wavenet/model.cpp @@ -8,6 +8,7 @@ #include #include "../get_dsp.h" +#include "../json_util.h" #include "../registry.h" #include "slimmable.h" #include "model.h" @@ -851,14 +852,29 @@ nam::wavenet::WaveNetConfig nam::wavenet::parse_config_json(const nlohmann::json } } - for (size_t i = 0; i < config["layers"].size(); i++) + const nlohmann::json& layers_json = nam::util::RequireField(config, "layers", "WaveNet config"); + if (!layers_json.is_array()) { - nlohmann::json layer_config = config["layers"][i]; + throw std::runtime_error("WaveNet config: 'layers' must be an array"); + } + if (layers_json.size() > static_cast(nam::util::kMaxModelArrayLength)) + { + throw std::runtime_error("WaveNet config: 'layers' has " + std::to_string(layers_json.size()) + + " elements, which exceeds the limit of " + + std::to_string(nam::util::kMaxModelArrayLength)); + } + for (size_t i = 0; i < layers_json.size(); i++) + { + nlohmann::json layer_config = layers_json[i]; + const std::string layer_context = "WaveNet layer array " + std::to_string(i); - const int groups = layer_config.value("groups_input", 1); // defaults to 1 - const int groups_input_mixin = layer_config.value("groups_input_mixin", 1); // defaults to 1 + // "groups_input"/"groups_input_mixin" default to 1, but a present-and-hostile value (e.g. + // 0) would divide-by-zero downstream (channels % groups), so validate when present. + const int groups = nam::util::OptionalDimension(layer_config, "groups_input", layer_context.c_str(), 1); + const int groups_input_mixin = + nam::util::OptionalDimension(layer_config, "groups_input_mixin", layer_context.c_str(), 1); - const int channels = layer_config["channels"]; + const int channels = nam::util::RequireDimension(layer_config, "channels", layer_context.c_str()); const int bottleneck = layer_config.value("bottleneck", channels); // defaults to channels if not present // Parse layer1x1 parameters @@ -866,14 +882,15 @@ nam::wavenet::WaveNetConfig nam::wavenet::parse_config_json(const nlohmann::json int layer1x1_groups = 1; if (layer_config.find("layer1x1") != layer_config.end()) { - const auto& layer1x1_config = layer_config["layer1x1"]; - layer1x1_active = layer1x1_config["active"]; - layer1x1_groups = layer1x1_config["groups"]; + const nlohmann::json& layer1x1_config = layer_config["layer1x1"]; + const std::string layer1x1_context = layer_context + ".layer1x1"; + layer1x1_active = nam::util::RequireValue(layer1x1_config, "active", layer1x1_context.c_str()); + layer1x1_groups = nam::util::RequireDimension(layer1x1_config, "groups", layer1x1_context.c_str()); } nam::wavenet::Layer1x1Params layer1x1_params(layer1x1_active, layer1x1_groups); - const int input_size = layer_config["input_size"]; - const int condition_size = layer_config["condition_size"]; + const int input_size = nam::util::RequireDimension(layer_config, "input_size", layer_context.c_str()); + const int condition_size = nam::util::RequireDimension(layer_config, "condition_size", layer_context.c_str()); int head_size = 0; int head_dilation = 1; @@ -883,26 +900,27 @@ nam::wavenet::WaveNetConfig nam::wavenet::parse_config_json(const nlohmann::json // Prefer nested "head" (matches trainer export). Legacy .nam uses head_size + head_bias (implicit kernel 1). if (layer_config.find("head") != layer_config.end() && !layer_config["head"].is_null()) { - const auto& head_json = layer_config["head"]; + const nlohmann::json& head_json = layer_config["head"]; if (!head_json.is_object()) { throw std::runtime_error("Layer array " + std::to_string(i) + ": 'head' must be a JSON object"); } - head_size = head_json.at("out_channels").get(); + const std::string head_context = layer_context + ".head"; + head_size = nam::util::RequireDimension(head_json, "out_channels", head_context.c_str()); if (head_json.contains("head_dilation")) { - head_dilation = head_json.at("head_dilation").get(); + head_dilation = nam::util::RequireDimension(head_json, "head_dilation", head_context.c_str()); } - head_kernel_size = head_json.at("kernel_size").get(); - head_bias = head_json.at("bias").get(); + head_kernel_size = nam::util::RequireDimension(head_json, "kernel_size", head_context.c_str()); + head_bias = nam::util::RequireValue(head_json, "bias", head_context.c_str()); } else if (layer_config.find("head_size") != layer_config.end()) { - head_size = layer_config["head_size"].get(); + head_size = nam::util::RequireDimension(layer_config, "head_size", layer_context.c_str()); head_kernel_size = 1; - head_bias = layer_config.at("head_bias").get(); + head_bias = nam::util::RequireValue(layer_config, "head_bias", layer_context.c_str()); } else { @@ -916,7 +934,8 @@ nam::wavenet::WaveNetConfig nam::wavenet::parse_config_json(const nlohmann::json throw std::runtime_error("Layer array " + std::to_string(i) + ": head.kernel_size must be >= 1"); } - const auto dilations = layer_config["dilations"]; + const std::vector dilations = nam::util::RequireIntArray( + layer_config, "dilations", layer_context.c_str(), 1, nam::util::kMaxModelDimension, /*allowEmpty=*/false); const size_t num_layers = dilations.size(); // Parse kernel sizes - support legacy single-value kernel_size or new per-layer kernel_sizes @@ -930,15 +949,11 @@ nam::wavenet::WaveNetConfig nam::wavenet::parse_config_json(const nlohmann::json } else if (has_kernel_sizes) { - const auto& kernel_sizes_json = layer_config["kernel_sizes"]; - if (!kernel_sizes_json.is_array()) - { - throw std::runtime_error("Layer array " + std::to_string(i) + ": kernel_sizes must be an array"); - } - for (const auto& ks_json : kernel_sizes_json) - { - kernel_sizes.push_back(ks_json.get()); - } + // A negative/zero kernel size becomes a huge size_t in a downstream resize(); require + // >= 1, like head.kernel_size above. allowEmpty is left true here because emptiness (vs. + // the required dilations-length match) is checked explicitly below. + kernel_sizes = nam::util::RequireIntArray( + layer_config, "kernel_sizes", layer_context.c_str(), 1, nam::util::kMaxModelDimension, /*allowEmpty=*/true); if (kernel_sizes.size() != num_layers) { throw std::runtime_error("Layer array " + std::to_string(i) + ": kernel_sizes array size (" @@ -948,7 +963,7 @@ nam::wavenet::WaveNetConfig nam::wavenet::parse_config_json(const nlohmann::json } else if (has_kernel_size) { - const int kernel_size = layer_config["kernel_size"].get(); + const int kernel_size = nam::util::RequireDimension(layer_config, "kernel_size", layer_context.c_str()); kernel_sizes.resize(num_layers, kernel_size); } else @@ -959,9 +974,11 @@ nam::wavenet::WaveNetConfig nam::wavenet::parse_config_json(const nlohmann::json // Parse activation config(s) - support both single config and array std::vector activation_configs; - if (layer_config["activation"].is_array()) + const nlohmann::json& activation_json_field = + nam::util::RequireField(layer_config, "activation", layer_context.c_str()); + if (activation_json_field.is_array()) { - for (const auto& activation_json : layer_config["activation"]) + for (const auto& activation_json : activation_json_field) { activation_configs.push_back(activations::ActivationConfig::from_json(activation_json)); } @@ -976,7 +993,7 @@ nam::wavenet::WaveNetConfig nam::wavenet::parse_config_json(const nlohmann::json { // Single activation config - duplicate it for all layers const activations::ActivationConfig activation_config = - activations::ActivationConfig::from_json(layer_config["activation"]); + activations::ActivationConfig::from_json(activation_json_field); activation_configs.resize(num_layers, activation_config); } @@ -1113,15 +1130,16 @@ nam::wavenet::WaveNetConfig nam::wavenet::parse_config_json(const nlohmann::json int head1x1_groups = 1; if (layer_config.find("head1x1") != layer_config.end()) { - const auto& head1x1_config = layer_config["head1x1"]; - head1x1_active = head1x1_config["active"]; - head1x1_out_channels = head1x1_config["out_channels"]; - head1x1_groups = head1x1_config["groups"]; + const nlohmann::json& head1x1_config = layer_config["head1x1"]; + const std::string head1x1_context = layer_context + ".head1x1"; + head1x1_active = nam::util::RequireValue(head1x1_config, "active", head1x1_context.c_str()); + head1x1_out_channels = nam::util::RequireDimension(head1x1_config, "out_channels", head1x1_context.c_str()); + head1x1_groups = nam::util::RequireDimension(head1x1_config, "groups", head1x1_context.c_str()); } nam::wavenet::Head1x1Params head1x1_params(head1x1_active, head1x1_out_channels, head1x1_groups); // Helper function to parse FiLM parameters - auto parse_film_params = [&layer_config](const std::string& key) -> nam::wavenet::_FiLMParams { + auto parse_film_params = [&layer_config, &layer_context](const std::string& key) -> nam::wavenet::_FiLMParams { if (layer_config.find(key) == layer_config.end() || layer_config[key] == false) { return nam::wavenet::_FiLMParams(false, false); @@ -1129,7 +1147,8 @@ nam::wavenet::WaveNetConfig nam::wavenet::parse_config_json(const nlohmann::json const nlohmann::json& film_config = layer_config[key]; bool active = film_config.value("active", true); bool shift = film_config.value("shift", true); - int film_groups = film_config.value("groups", 1); + // Validated for the same reason as groups_input: a zero here reaches `x % groups` downstream. + int film_groups = nam::util::OptionalDimension(film_config, "groups", layer_context.c_str(), 1); return nam::wavenet::_FiLMParams(active, shift, film_groups); }; @@ -1151,16 +1170,17 @@ nam::wavenet::WaveNetConfig nam::wavenet::parse_config_json(const nlohmann::json } wc.layer_array_params.push_back(nam::wavenet::LayerArrayParams( - input_size, condition_size, head_size, head_dilation, head_kernel_size, channels, bottleneck, std::move(kernel_sizes), dilations, - std::move(activation_configs), std::move(gating_modes), head_bias, groups, groups_input_mixin, layer1x1_params, - head1x1_params, std::move(secondary_activation_configs), conv_pre_film_params, conv_post_film_params, - input_mixin_pre_film_params, input_mixin_post_film_params, activation_pre_film_params, - activation_post_film_params, _layer1x1_post_film_params, head1x1_post_film_params)); + input_size, condition_size, head_size, head_dilation, head_kernel_size, channels, bottleneck, + std::move(kernel_sizes), std::move(dilations), std::move(activation_configs), std::move(gating_modes), head_bias, + groups, groups_input_mixin, layer1x1_params, head1x1_params, std::move(secondary_activation_configs), + conv_pre_film_params, conv_post_film_params, input_mixin_pre_film_params, input_mixin_post_film_params, + activation_pre_film_params, activation_post_film_params, _layer1x1_post_film_params, head1x1_post_film_params)); } wc.with_head = config.find("head") != config.end() && !config["head"].is_null(); - wc.head_scale = config["head_scale"]; - wc.in_channels = config.value("in_channels", 1); + wc.head_scale = nam::util::RequireValue(config, "head_scale", "WaveNet config"); + // Optional, defaults to 1, but a present-and-hostile value feeds a buffer resize()--validate it. + wc.in_channels = nam::util::OptionalDimension(config, "in_channels", "WaveNet config", 1); if (wc.layer_array_params.empty()) throw std::runtime_error("WaveNet config requires at least one layer array"); @@ -1183,10 +1203,13 @@ nam::wavenet::WaveNetConfig nam::wavenet::parse_config_json(const nlohmann::json } } hp.in_channels = implied_in; - hp.channels = hj.at("channels").get(); - hp.out_channels = hj.at("out_channels").get(); - hp.kernel_sizes = hj.at("kernel_sizes").get>(); - hp.activation_config = nam::activations::ActivationConfig::from_json(hj.at("activation")); + static constexpr const char* kHeadContext = "WaveNet config head"; + hp.channels = nam::util::RequireDimension(hj, "channels", kHeadContext); + hp.out_channels = nam::util::RequireDimension(hj, "out_channels", kHeadContext); + hp.kernel_sizes = nam::util::RequireIntArray(hj, "kernel_sizes", kHeadContext, 1, nam::util::kMaxModelDimension, + /*allowEmpty=*/true); + hp.activation_config = + nam::activations::ActivationConfig::from_json(nam::util::RequireField(hj, "activation", kHeadContext)); if (hp.kernel_sizes.empty()) throw std::runtime_error("WaveNet config: head.kernel_sizes must be non-empty"); wc.head_params = std::move(hp); diff --git a/tools/run_tests.cpp b/tools/run_tests.cpp index 5699bf78..048a3735 100644 --- a/tools/run_tests.cpp +++ b/tools/run_tests.cpp @@ -28,6 +28,7 @@ #include "test/test_input_buffer_verification.cpp" #include "test/test_linear.cpp" #include "test/test_lstm.cpp" +#include "test/test_model_validation.cpp" #include "test/test_lstm_realtime_safe.cpp" #include "test/test_wavenet_configurable_gating.cpp" #include "test/test_noncontiguous_blocks.cpp" @@ -313,6 +314,29 @@ int main() // Finally, some end-to-end tests. test_get_dsp::test_load_and_process_nam_files(); + // Model-file validation: malformed .nam configs must throw std::runtime_error, not + // trigger undefined behavior (see NAM/json_util.h). + test_model_validation::test_missing_version_throws(); + test_model_validation::test_missing_architecture_throws(); + test_model_validation::test_missing_config_throws(); + test_model_validation::test_wavenet_missing_layers_throws(); + test_model_validation::test_wavenet_negative_channels_throws(); + test_model_validation::test_wavenet_absurdly_large_channels_throws(); + test_model_validation::test_wavenet_empty_dilations_throws(); + test_model_validation::test_linear_missing_receptive_field_throws(); + test_model_validation::test_wavenet_empty_layer1x1_throws(); + test_model_validation::test_wavenet_head1x1_missing_fields_throws(); + test_model_validation::test_wavenet_activation_missing_type_throws(); + test_model_validation::test_wavenet_zero_groups_input_throws(); + test_model_validation::test_wavenet_zero_film_groups_throws(); + test_model_validation::test_wavenet_negative_kernel_size_throws(); + test_model_validation::test_wavenet_overlong_dilations_throws(); + test_model_validation::test_wavenet_non_integral_channels_throws(); + test_model_validation::test_dimension_boundary_at_max_accepted(); + test_model_validation::test_dimension_boundary_beyond_max_throws(); + test_model_validation::test_valid_wavenet_config_still_loads(); + test_model_validation::test_valid_linear_config_still_loads(); + // Extensibility: external architecture registration and get_dsp (issue #230) test_extensible::run_extensibility_tests(); diff --git a/tools/test/test_model_validation.cpp b/tools/test/test_model_validation.cpp new file mode 100644 index 00000000..725caddf --- /dev/null +++ b/tools/test/test_model_validation.cpp @@ -0,0 +1,379 @@ +// Tests that malformed `.nam` model files are rejected with std::runtime_error instead of +// hitting undefined behavior (see NAM/json_util.h). `.nam` files are untrusted input +// downloaded from the internet, so every field the loader treats as required must fail +// safely when it is missing or out of range. + +#include +#include +#include +#include +#include + +#include "json.hpp" + +#include "NAM/get_dsp.h" +#include "NAM/json_util.h" + +namespace test_model_validation +{ +namespace +{ + +// A minimal, known-good WaveNet model (same shape as the one in +// test_wavenet::test_factory::test_factory_without_head_key), used as a baseline that +// individual tests mutate to introduce exactly one defect. +nlohmann::json build_valid_wavenet_config() +{ + const std::string configStr = R"({ + "version": "0.5.4", + "metadata": {}, + "architecture": "WaveNet", + "config": { + "layers": [{ + "input_size": 1, + "condition_size": 1, + "head_size": 1, + "channels": 1, + "kernel_size": 1, + "dilations": [1], + "activation": "ReLU", + "gated": false, + "head_bias": false + }], + "head_scale": 1.0 + }, + "weights": [1.0, 1.0, 0.0, 1.0, 1.0, 0.0, 1.0, 1.0], + "sample_rate": 48000 + })"; + return nlohmann::json::parse(configStr); +} + +// A minimal, known-good Linear model. +nlohmann::json build_valid_linear_config() +{ + const std::string configStr = R"({ + "version": "0.5.4", + "metadata": {}, + "architecture": "Linear", + "config": {"receptive_field": 4, "bias": true}, + "weights": [0.1, 0.2, 0.3, 0.4, 0.05], + "sample_rate": 48000 + })"; + return nlohmann::json::parse(configStr); +} + +} // namespace + +void test_missing_version_throws() +{ + nlohmann::json j = build_valid_wavenet_config(); + j.erase("version"); + try + { + nam::get_dsp(j); + assert(false && "should have thrown"); + } + catch (const std::runtime_error&) + { + } +} + +void test_missing_architecture_throws() +{ + nlohmann::json j = build_valid_wavenet_config(); + j.erase("architecture"); + try + { + nam::get_dsp(j); + assert(false && "should have thrown"); + } + catch (const std::runtime_error&) + { + } +} + +void test_missing_config_throws() +{ + nlohmann::json j = build_valid_wavenet_config(); + j.erase("config"); + try + { + nam::get_dsp(j); + assert(false && "should have thrown"); + } + catch (const std::runtime_error&) + { + } +} + +void test_wavenet_missing_layers_throws() +{ + nlohmann::json j = build_valid_wavenet_config(); + j["config"].erase("layers"); + try + { + nam::get_dsp(j); + assert(false && "should have thrown"); + } + catch (const std::runtime_error&) + { + } +} + +void test_wavenet_negative_channels_throws() +{ + nlohmann::json j = build_valid_wavenet_config(); + j["config"]["layers"][0]["channels"] = -1; + try + { + nam::get_dsp(j); + assert(false && "should have thrown"); + } + catch (const std::runtime_error&) + { + } +} + +void test_wavenet_absurdly_large_channels_throws() +{ + nlohmann::json j = build_valid_wavenet_config(); + j["config"]["layers"][0]["channels"] = 100000000; + try + { + nam::get_dsp(j); + assert(false && "should have thrown"); + } + catch (const std::runtime_error&) + { + } +} + +void test_wavenet_empty_dilations_throws() +{ + nlohmann::json j = build_valid_wavenet_config(); + j["config"]["layers"][0]["dilations"] = nlohmann::json::array(); + try + { + nam::get_dsp(j); + assert(false && "should have thrown"); + } + catch (const std::runtime_error&) + { + } +} + +void test_linear_missing_receptive_field_throws() +{ + nlohmann::json j = build_valid_linear_config(); + j["config"].erase("receptive_field"); + try + { + nam::get_dsp(j); + assert(false && "should have thrown"); + } + catch (const std::runtime_error&) + { + } +} + +// "layer1x1" present but empty must throw instead of hitting nlohmann's const-operator[] +// assert (JSON_ASSERT / UB under -DNDEBUG) when reading "active"/"groups". +void test_wavenet_empty_layer1x1_throws() +{ + nlohmann::json j = build_valid_wavenet_config(); + j["config"]["layers"][0]["layer1x1"] = nlohmann::json::object(); + try + { + nam::get_dsp(j); + assert(false && "should have thrown"); + } + catch (const std::runtime_error&) + { + } +} + +// "head1x1" present with only "active" (missing "out_channels"/"groups") must throw instead +// of hitting the same const-operator[] UB. +void test_wavenet_head1x1_missing_fields_throws() +{ + nlohmann::json j = build_valid_wavenet_config(); + j["config"]["layers"][0]["head1x1"] = {{"active", true}}; + try + { + nam::get_dsp(j); + assert(false && "should have thrown"); + } + catch (const std::runtime_error&) + { + } +} + +// An "activation" object missing its "type" field must throw instead of hitting the same +// const-operator[] UB in ActivationConfig::from_json. +void test_wavenet_activation_missing_type_throws() +{ + nlohmann::json j = build_valid_wavenet_config(); + j["config"]["layers"][0]["activation"] = nlohmann::json::object(); + try + { + nam::get_dsp(j); + assert(false && "should have thrown"); + } + catch (const std::runtime_error&) + { + } +} + +// "groups_input": 0 must be rejected, not divide-by-zero downstream (channels % groups). +void test_wavenet_zero_groups_input_throws() +{ + nlohmann::json j = build_valid_wavenet_config(); + j["config"]["layers"][0]["groups_input"] = 0; + try + { + nam::get_dsp(j); + assert(false && "should have thrown"); + } + catch (const std::runtime_error&) + { + } +} + +// A FiLM block's "groups" reaches the same `x % groups` as groups_input, so zero must be rejected too. +void test_wavenet_zero_film_groups_throws() +{ + nlohmann::json j = build_valid_wavenet_config(); + j["config"]["layers"][0]["conv_pre_film"] = {{"active", true}, {"shift", true}, {"groups", 0}}; + try + { + nam::get_dsp(j); + assert(false && "should have thrown"); + } + catch (const std::runtime_error&) + { + } +} + +// A negative kernel_size must be rejected, not become a huge size_t in a downstream resize(). +void test_wavenet_negative_kernel_size_throws() +{ + nlohmann::json j = build_valid_wavenet_config(); + j["config"]["layers"][0]["kernel_size"] = -1; + try + { + nam::get_dsp(j); + assert(false && "should have thrown"); + } + catch (const std::runtime_error&) + { + } +} + +// An over-long "dilations" array (beyond nam::util::kMaxModelArrayLength) must be rejected-- +// each element drives construction of a heap-allocated Layer object. +void test_wavenet_overlong_dilations_throws() +{ + nlohmann::json j = build_valid_wavenet_config(); + nlohmann::json dilations = nlohmann::json::array(); + for (int i = 0; i < 5000; i++) + { + dilations.push_back(1); + } + j["config"]["layers"][0]["dilations"] = dilations; + try + { + nam::get_dsp(j); + assert(false && "should have thrown"); + } + catch (const std::runtime_error&) + { + } +} + +// A non-integral dimension (a JSON float) must be rejected rather than silently truncated. +void test_wavenet_non_integral_channels_throws() +{ + nlohmann::json j = build_valid_wavenet_config(); + j["config"]["layers"][0]["channels"] = 4.9; + try + { + nam::get_dsp(j); + assert(false && "should have thrown"); + } + catch (const std::runtime_error&) + { + } +} + +// Boundary tests for nam::util::RequireDimension, exercised directly rather than through a +// full model load: a model with channels == kMaxModelDimension would try to allocate +// matrices on the order of kMaxModelDimension^2 floats, which isn't a reasonable thing for a +// unit test to do just to exercise a boundary condition. +void test_dimension_boundary_at_max_accepted() +{ + const nlohmann::json j = {{"channels", nam::util::kMaxModelDimension}}; + const int value = nam::util::RequireDimension(j, "channels", "boundary test"); + assert(value == nam::util::kMaxModelDimension); +} + +void test_dimension_boundary_beyond_max_throws() +{ + const nlohmann::json j = {{"channels", nam::util::kMaxModelDimension + 1}}; + try + { + nam::util::RequireDimension(j, "channels", "boundary test"); + assert(false && "should have thrown"); + } + catch (const std::runtime_error&) + { + } +} + +// A well-formed WaveNet config must still load and process audio unchanged. +void test_valid_wavenet_config_still_loads() +{ + nlohmann::json j = build_valid_wavenet_config(); + std::unique_ptr dsp = nam::get_dsp(j); + assert(dsp != nullptr); + + const int numFrames = 4; + const int maxBufferSize = 64; + dsp->Reset(48000.0, maxBufferSize); + + std::vector input(numFrames, 1.0f); + std::vector output(numFrames, 0.0f); + NAM_SAMPLE* inputPtrs[] = {input.data()}; + NAM_SAMPLE* outputPtrs[] = {output.data()}; + + dsp->process(inputPtrs, outputPtrs, numFrames); + + for (int i = 0; i < numFrames; i++) + { + assert(std::isfinite(output[i])); + } +} + +// A well-formed Linear config must still load and process audio unchanged. +void test_valid_linear_config_still_loads() +{ + nlohmann::json j = build_valid_linear_config(); + std::unique_ptr dsp = nam::get_dsp(j); + assert(dsp != nullptr); + + const int numFrames = 4; + const int maxBufferSize = 64; + dsp->Reset(48000.0, maxBufferSize); + + std::vector input(numFrames, 1.0f); + std::vector output(numFrames, 0.0f); + NAM_SAMPLE* inputPtrs[] = {input.data()}; + NAM_SAMPLE* outputPtrs[] = {output.data()}; + + dsp->process(inputPtrs, outputPtrs, numFrames); + + for (int i = 0; i < numFrames; i++) + { + assert(std::isfinite(output[i])); + } +} +}; // namespace test_model_validation