From 69ab39d6585b8cd2ffb2618ece3e4867bb59b9e6 Mon Sep 17 00:00:00 2001 From: Olek <115580134+oleks-rip@users.noreply.github.com> Date: Thu, 18 Dec 2025 14:13:48 -0500 Subject: [PATCH] Fix potential memory leaks found by srlabs (#6145) --- src/xrpld/app/wasm/WasmiVM.h | 140 +++++++++----- src/xrpld/app/wasm/detail/WasmiVM.cpp | 257 ++++++++++++-------------- 2 files changed, 209 insertions(+), 188 deletions(-) diff --git a/src/xrpld/app/wasm/WasmiVM.h b/src/xrpld/app/wasm/WasmiVM.h index a9b8a94332..b4148b7dea 100644 --- a/src/xrpld/app/wasm/WasmiVM.h +++ b/src/xrpld/app/wasm/WasmiVM.h @@ -7,42 +7,93 @@ namespace ripple { -struct WasmiResult +template +struct WasmVec { - wasm_val_vec_t r; - bool f; // failure flag + T vec_; - WasmiResult(unsigned N = 0) : r{0, nullptr}, f(false) + WasmVec(size_t s = 0) : vec_ WASM_EMPTY_VEC { - if (N) - wasm_val_vec_new_uninitialized(&r, N); + if (s > 0) + Create(&vec_, s); // zeroes memory } - ~WasmiResult() + ~WasmVec() { - if (r.size) - wasm_val_vec_delete(&r); + clear(); } - WasmiResult(WasmiResult const&) = delete; - WasmiResult& - operator=(WasmiResult const&) = delete; + WasmVec(WasmVec const&) = delete; + WasmVec& + operator=(WasmVec const&) = delete; - WasmiResult(WasmiResult&& o) + WasmVec(WasmVec&& other) noexcept : vec_ WASM_EMPTY_VEC { - *this = std::move(o); + *this = std::move(other); } - WasmiResult& - operator=(WasmiResult&& o) + WasmVec& + operator=(WasmVec&& other) noexcept { - r = o.r; - o.r = {0, nullptr}; - f = o.f; - o.f = false; + if (this != &other) + { + clear(); + vec_ = other.vec_; + other.vec_ = WASM_EMPTY_VEC; + } return *this; } - // operator wasm_val_vec_t &() {return r;} + + void + clear() + { + Destroy(&vec_); // call destructor for every elements too + vec_ = WASM_EMPTY_VEC; + } + + T + release() + { + T result = vec_; + vec_ = WASM_EMPTY_VEC; + return result; + } +}; + +using WasmValtypeVec = WasmVec< + wasm_valtype_vec_t, + &wasm_valtype_vec_new_uninitialized, + &wasm_valtype_vec_delete>; +using WasmValVec = WasmVec< + wasm_val_vec_t, + &wasm_val_vec_new_uninitialized, + &wasm_val_vec_delete>; +using WasmExternVec = WasmVec< + wasm_extern_vec_t, + &wasm_extern_vec_new_uninitialized, + &wasm_extern_vec_delete>; +using WasmExporttypeVec = WasmVec< + wasm_exporttype_vec_t, + &wasm_exporttype_vec_new_uninitialized, + &wasm_exporttype_vec_delete>; +using WasmImporttypeVec = WasmVec< + wasm_importtype_vec_t, + &wasm_importtype_vec_new_uninitialized, + &wasm_importtype_vec_delete>; + +struct WasmiResult +{ + WasmValVec r; + bool f; // failure flag + + WasmiResult(unsigned N = 0) : r(N), f(false) + { + } + + ~WasmiResult() = default; + WasmiResult(WasmiResult&& o) = default; + WasmiResult& + operator=(WasmiResult&& o) = default; }; using ModulePtr = std::unique_ptr; @@ -56,17 +107,17 @@ using FuncInfo = std::pair; struct InstanceWrapper { wasm_store_t* store_ = nullptr; - wasm_extern_vec_t exports_; + WasmExternVec exports_; InstancePtr instance_; beast::Journal j_ = beast::Journal(beast::Journal::getNullSink()); private: static InstancePtr init( - wasm_store_t* s, - wasm_module_t* m, - wasm_extern_vec_t* expt, - wasm_extern_vec_t const& imports, + StorePtr& s, + ModulePtr& m, + WasmExternVec& expt, + WasmExternVec const& imports, beast::Journal j); public: @@ -78,19 +129,18 @@ public: operator=(InstanceWrapper&& o); InstanceWrapper( - wasm_store_t* s, - wasm_module_t* m, - wasm_extern_vec_t const& imports, + StorePtr& s, + ModulePtr& m, + WasmExternVec const& imports, beast::Journal j); - ~InstanceWrapper(); + ~InstanceWrapper() = default; operator bool() const; FuncInfo - getFunc( - std::string_view funcName, - wasm_exporttype_vec_t const& export_types) const; + getFunc(std::string_view funcName, WasmExporttypeVec const& exportTypes) + const; wmem getMem() const; @@ -105,12 +155,12 @@ struct ModuleWrapper { ModulePtr module_; InstanceWrapper instanceWrap_; - wasm_exporttype_vec_t exportTypes_; + WasmExporttypeVec exportTypes_; beast::Journal j_ = beast::Journal(beast::Journal::getNullSink()); private: static ModulePtr - init(wasm_store_t* s, Bytes const& wasmBin, beast::Journal j); + init(StorePtr& s, Bytes const& wasmBin, beast::Journal j); public: ModuleWrapper(); @@ -118,12 +168,12 @@ public: ModuleWrapper& operator=(ModuleWrapper&& o); ModuleWrapper( - wasm_store_t* s, + StorePtr& s, Bytes const& wasmBin, bool instantiate, ImportVec const& imports, beast::Journal j); - ~ModuleWrapper(); + ~ModuleWrapper() = default; operator bool() const; @@ -136,20 +186,14 @@ public: getInstance(int i = 0) const; int - addInstance( - wasm_store_t* s, - wasm_extern_vec_t const& imports = WASM_EMPTY_VEC); + addInstance(StorePtr& s, WasmExternVec const& imports); std::int64_t getGas(); private: - static void - makeImpParams(wasm_valtype_vec_t& v, WasmImportFunc const& imp); - static void - makeImpReturn(wasm_valtype_vec_t& v, WasmImportFunc const& imp); - wasm_extern_vec_t - buildImports(wasm_store_t* s, ImportVec const& imports); + WasmExternVec + buildImports(StorePtr& s, ImportVec const& imports); }; class WasmiEngine @@ -237,9 +281,7 @@ private: runFunc(std::string_view const funcName, int32_t p); int32_t - makeModule( - Bytes const& wasmCode, - wasm_extern_vec_t const& imports = WASM_EMPTY_VEC); + makeModule(Bytes const& wasmCode, WasmExternVec const& imports = {}); FuncInfo getFunc(std::string_view funcName); diff --git a/src/xrpld/app/wasm/detail/WasmiVM.cpp b/src/xrpld/app/wasm/detail/WasmiVM.cpp index 6e79827060..1a37910105 100644 --- a/src/xrpld/app/wasm/detail/WasmiVM.cpp +++ b/src/xrpld/app/wasm/detail/WasmiVM.cpp @@ -51,56 +51,47 @@ print_wasm_error(std::string_view msg, wasm_trap_t* trap, beast::Journal jlog) InstancePtr InstanceWrapper::init( - wasm_store_t* s, - wasm_module_t* m, - wasm_extern_vec_t* expt, - wasm_extern_vec_t const& imports, + StorePtr& s, + ModulePtr& m, + WasmExternVec& expt, + WasmExternVec const& imports, beast::Journal j) { wasm_trap_t* trap = nullptr; InstancePtr mi = InstancePtr( - wasm_instance_new(s, m, &imports, &trap), &wasm_instance_delete); + wasm_instance_new(s.get(), m.get(), &imports.vec_, &trap), + &wasm_instance_delete); if (!mi || trap) { print_wasm_error("can't create instance", trap, j); throw std::runtime_error("can't create instance"); } - wasm_instance_exports(mi.get(), expt); + wasm_instance_exports(mi.get(), &expt.vec_); return mi; } -InstanceWrapper::InstanceWrapper() - : exports_{0, nullptr}, instance_(nullptr, &wasm_instance_delete) +InstanceWrapper::InstanceWrapper() : instance_(nullptr, &wasm_instance_delete) { } // LCOV_EXCL_START InstanceWrapper::InstanceWrapper(InstanceWrapper&& o) - : exports_{0, nullptr}, instance_(nullptr, &wasm_instance_delete) + : instance_(nullptr, &wasm_instance_delete) { *this = std::move(o); } // LCOV_EXCL_STOP InstanceWrapper::InstanceWrapper( - wasm_store_t* s, - wasm_module_t* m, - wasm_extern_vec_t const& imports, + StorePtr& s, + ModulePtr& m, + WasmExternVec const& imports, beast::Journal j) - : store_(s) - , exports_ WASM_EMPTY_VEC - , instance_(init(s, m, &exports_, imports, j)) - , j_(j) + : store_(s.get()), instance_(init(s, m, exports_, imports, j)), j_(j) { } -InstanceWrapper::~InstanceWrapper() -{ - if (exports_.size) - wasm_extern_vec_delete(&exports_); -} - InstanceWrapper& InstanceWrapper::operator=(InstanceWrapper&& o) { @@ -109,11 +100,7 @@ InstanceWrapper::operator=(InstanceWrapper&& o) store_ = o.store_; o.store_ = nullptr; - if (exports_.size) - wasm_extern_vec_delete(&exports_); // LCOV_EXCL_LINE - exports_ = o.exports_; - o.exports_ = {0, nullptr}; - + exports_ = std::move(o.exports_); instance_ = std::move(o.instance_); j_ = o.j_; @@ -129,39 +116,37 @@ InstanceWrapper::operator bool() const FuncInfo InstanceWrapper::getFunc( std::string_view funcName, - wasm_exporttype_vec_t const& export_types) const + WasmExporttypeVec const& exportTypes) const { - wasm_func_t* f = nullptr; - wasm_functype_t* ft = nullptr; + wasm_func_t const* f = nullptr; + wasm_functype_t const* ft = nullptr; if (!instance_) throw std::runtime_error("no instance"); // LCOV_EXCL_LINE - if (!export_types.size) + if (!exportTypes.vec_.size) throw std::runtime_error("no export"); // LCOV_EXCL_LINE - if (export_types.size != exports_.size) + if (exportTypes.vec_.size != exports_.vec_.size) throw std::runtime_error("invalid export"); // LCOV_EXCL_LINE - for (unsigned i = 0; i < export_types.size; ++i) + for (unsigned i = 0; i < exportTypes.vec_.size; ++i) { - auto const* exp_type(export_types.data[i]); + auto const* expType(exportTypes.vec_.data[i]); - wasm_name_t const* name = wasm_exporttype_name(exp_type); - wasm_externtype_t const* exn_type = wasm_exporttype_type(exp_type); - if (wasm_externtype_kind(exn_type) == WASM_EXTERN_FUNC) + wasm_name_t const* name = wasm_exporttype_name(expType); + wasm_externtype_t const* exnType = wasm_exporttype_type(expType); + if (wasm_externtype_kind(exnType) == WASM_EXTERN_FUNC) { - if (funcName == std::string_view(name->data, name->size)) - { - auto* exn(exports_.data[i]); - if (wasm_extern_kind(exn) != WASM_EXTERN_FUNC) - throw std::runtime_error( - "invalid export"); // LCOV_EXCL_LINE + if (funcName != std::string_view(name->data, name->size)) + continue; - ft = wasm_externtype_as_functype( - const_cast(exn_type)); - f = wasm_extern_as_func(exn); - break; - } + auto const* exn(exports_.vec_.data[i]); + if (wasm_extern_kind(exn) != WASM_EXTERN_FUNC) + throw std::runtime_error("invalid export"); // LCOV_EXCL_LINE + + ft = wasm_externtype_as_functype_const(exnType); + f = wasm_extern_as_func_const(exn); + break; } } @@ -179,9 +164,9 @@ InstanceWrapper::getMem() const throw std::runtime_error("no instance"); // LCOV_EXCL_LINE wasm_memory_t* mem = nullptr; - for (unsigned i = 0; i < exports_.size; ++i) + for (unsigned i = 0; i < exports_.vec_.size; ++i) { - auto* e(exports_.data[i]); + auto* e(exports_.vec_.data[i]); if (wasm_extern_kind(e) == WASM_EXTERN_MEMORY) { mem = wasm_extern_as_memory(e); @@ -219,9 +204,11 @@ InstanceWrapper::setGas(std::int64_t gas) const wasm_store_set_fuel(store_, static_cast(gas)); if (err) { - print_wasm_error( - "Can't set instance gas", nullptr, j_); // LCOV_EXCL_LINE - throw std::runtime_error("Can't set instance gas"); // LCOV_EXCL_LINE + // LCOV_EXCL_START + print_wasm_error("Can't set instance gas", nullptr, j_); + wasmi_error_delete(err); + throw std::runtime_error("Can't set instance gas"); + // LCOV_EXCL_STOP } return gas; @@ -230,10 +217,11 @@ InstanceWrapper::setGas(std::int64_t gas) const ////////////////////////////////////////////////////////////////////////////////////////////////////////////// ModulePtr -ModuleWrapper::init(wasm_store_t* s, Bytes const& wasmBin, beast::Journal j) +ModuleWrapper::init(StorePtr& s, Bytes const& wasmBin, beast::Journal j) { wasm_byte_vec_t const code{wasmBin.size(), (char*)(wasmBin.data())}; - ModulePtr m = ModulePtr(wasm_module_new(s, &code), &wasm_module_delete); + ModulePtr m = + ModulePtr(wasm_module_new(s.get(), &code), &wasm_module_delete); if (!m) throw std::runtime_error("can't create module"); @@ -241,41 +229,33 @@ ModuleWrapper::init(wasm_store_t* s, Bytes const& wasmBin, beast::Journal j) } // LCOV_EXCL_START -ModuleWrapper::ModuleWrapper() - : module_(nullptr, &wasm_module_delete), exportTypes_{0, nullptr} +ModuleWrapper::ModuleWrapper() : module_(nullptr, &wasm_module_delete) { } ModuleWrapper::ModuleWrapper(ModuleWrapper&& o) - : module_(nullptr, &wasm_module_delete), exportTypes_{0, nullptr} + : module_(nullptr, &wasm_module_delete) { *this = std::move(o); } // LCOV_EXCL_STOP ModuleWrapper::ModuleWrapper( - wasm_store_t* s, + StorePtr& s, Bytes const& wasmBin, bool instantiate, ImportVec const& imports, beast::Journal j) - : module_(init(s, wasmBin, j)), exportTypes_{0, nullptr}, j_(j) + : module_(init(s, wasmBin, j)), j_(j) { - wasm_module_exports(module_.get(), &exportTypes_); + wasm_module_exports(module_.get(), &exportTypes_.vec_); if (instantiate) { auto wimports = buildImports(s, imports); addInstance(s, wimports); - wasm_extern_vec_delete(&wimports); } } -ModuleWrapper::~ModuleWrapper() -{ - if (exportTypes_.size) - wasm_exporttype_vec_delete(&exportTypes_); -} - // LCOV_EXCL_START ModuleWrapper& ModuleWrapper::operator=(ModuleWrapper&& o) @@ -285,10 +265,7 @@ ModuleWrapper::operator=(ModuleWrapper&& o) module_ = std::move(o.module_); instanceWrap_ = std::move(o.instanceWrap_); - if (exportTypes_.size) - wasm_exporttype_vec_delete(&exportTypes_); - exportTypes_ = o.exportTypes_; - o.exportTypes_ = {0, nullptr}; + exportTypes_ = std::move(o.exportTypes_); j_ = o.j_; return *this; @@ -301,27 +278,25 @@ ModuleWrapper::operator bool() const // LCOV_EXCL_STOP -void -ModuleWrapper::makeImpParams(wasm_valtype_vec_t& v, WasmImportFunc const& imp) +static WasmValtypeVec +makeImpParams(WasmImportFunc const& imp) { auto const paramSize = imp.params.size(); + if (!paramSize) + return {}; + + WasmValtypeVec v(paramSize); - if (paramSize) - { - wasm_valtype_vec_new_uninitialized(&v, paramSize); - } - else - v = WASM_EMPTY_VEC; for (unsigned i = 0; i < paramSize; ++i) { auto const vt = imp.params[i]; switch (vt) { case WT_I32: - v.data[i] = wasm_valtype_new_i32(); + v.vec_.data[i] = wasm_valtype_new_i32(); break; case WT_I64: - v.data[i] = wasm_valtype_new_i64(); + v.vec_.data[i] = wasm_valtype_new_i64(); break; // LCOV_EXCL_START default: @@ -329,59 +304,55 @@ ModuleWrapper::makeImpParams(wasm_valtype_vec_t& v, WasmImportFunc const& imp) // LCOV_EXCL_STOP } } + return v; } -void -ModuleWrapper::makeImpReturn(wasm_valtype_vec_t& v, WasmImportFunc const& imp) +static WasmValtypeVec +makeImpReturn(WasmImportFunc const& imp) { - if (imp.result) + if (!imp.result) + return {}; // LCOV_EXCL_LINE + + WasmValtypeVec v(1); + switch (*imp.result) { - wasm_valtype_vec_new_uninitialized(&v, 1); - switch (*imp.result) - { - case WT_I32: - v.data[0] = wasm_valtype_new_i32(); - break; - // LCOV_EXCL_START - case WT_I64: - v.data[0] = wasm_valtype_new_i64(); - break; - default: - throw std::runtime_error("invalid return type"); - // LCOV_EXCL_STOP - } + case WT_I32: + v.vec_.data[0] = wasm_valtype_new_i32(); + break; + // LCOV_EXCL_START + case WT_I64: + v.vec_.data[0] = wasm_valtype_new_i64(); + break; + default: + throw std::runtime_error("invalid return type"); + // LCOV_EXCL_STOP } - else - v = WASM_EMPTY_VEC; // LCOV_EXCL_LINE + return v; } -wasm_extern_vec_t -ModuleWrapper::buildImports(wasm_store_t* s, ImportVec const& imports) +WasmExternVec +ModuleWrapper::buildImports(StorePtr& s, ImportVec const& imports) { - wasm_importtype_vec_t importTypes = WASM_EMPTY_VEC; - wasm_module_imports(module_.get(), &importTypes); - std:: - unique_ptr - itDeleter(&importTypes, &wasm_importtype_vec_delete); + WasmImporttypeVec importTypes; + wasm_module_imports(module_.get(), &importTypes.vec_); - wasm_extern_vec_t wimports = WASM_EMPTY_VEC; - if (!importTypes.size) - return wimports; + if (!importTypes.vec_.size) + return {}; - wasm_extern_vec_new_uninitialized(&wimports, importTypes.size); + WasmExternVec wimports(importTypes.vec_.size); unsigned impCnt = 0; - for (unsigned i = 0; i < importTypes.size; ++i) + for (unsigned i = 0; i < importTypes.vec_.size; ++i) { - wasm_importtype_t const* importtype = importTypes.data[i]; + wasm_importtype_t const* importType = importTypes.vec_.data[i]; // wasm_name_t const* mn = wasm_importtype_module(importtype); // auto modName = std::string_view(mn->data, mn->num_elems); - wasm_name_t const* fn = wasm_importtype_name(importtype); + wasm_name_t const* fn = wasm_importtype_name(importType); auto fieldName = std::string_view(fn->data, fn->size); wasm_externkind_t const itype = - wasm_externtype_kind(wasm_importtype_type(importtype)); + wasm_externtype_kind(wasm_importtype_type(importType)); if ((itype) != WASM_EXTERN_FUNC) throw std::runtime_error( "Invalid import type " + @@ -398,17 +369,19 @@ ModuleWrapper::buildImports(wasm_store_t* s, ImportVec const& imports) if (imp.name != fieldName) continue; - wasm_valtype_vec_t params = WASM_EMPTY_VEC, - results = WASM_EMPTY_VEC; - makeImpReturn(results, imp); - makeImpParams(params, imp); + WasmValtypeVec params(makeImpParams(imp)); + WasmValtypeVec results(makeImpReturn(imp)); + + std::unique_ptr + ftype( + wasm_functype_new(¶ms.vec_, &results.vec_), + &wasm_functype_delete); + + params.release(); + results.release(); - using ftype_ptr = std:: - unique_ptr; - ftype_ptr ftype( - wasm_functype_new(¶ms, &results), &wasm_functype_delete); wasm_func_t* func = wasm_func_new_with_env( - s, + s.get(), ftype.get(), reinterpret_cast(imp.wrap), (void*)&obj, @@ -421,7 +394,7 @@ ModuleWrapper::buildImports(wasm_store_t* s, ImportVec const& imports) // LCOV_EXCL_STOP } - wimports.data[i] = wasm_func_as_extern(func); + wimports.vec_.data[i] = wasm_func_as_extern(func); ++impCnt; impSet = true; @@ -435,11 +408,11 @@ ModuleWrapper::buildImports(wasm_store_t* s, ImportVec const& imports) } } - if (impCnt != importTypes.size) + if (impCnt != importTypes.vec_.size) { print_wasm_error( std::string("Imports not finished: ") + std::to_string(impCnt) + - "/" + std::to_string(importTypes.size), + "/" + std::to_string(importTypes.vec_.size), nullptr, j_); } @@ -466,9 +439,9 @@ ModuleWrapper::getInstance(int) const } int -ModuleWrapper::addInstance(wasm_store_t* s, wasm_extern_vec_t const& imports) +ModuleWrapper::addInstance(StorePtr& s, WasmExternVec const& imports) { - instanceWrap_ = {s, module_.get(), imports, j_}; + instanceWrap_ = {s, module_, imports, j_}; return 0; } @@ -547,12 +520,13 @@ WasmiEngine::addModule( { // LCOV_EXCL_START print_wasm_error("Error setting gas", nullptr, j_); + wasmi_error_delete(err); throw std::runtime_error("can't set gas"); // LCOV_EXCL_STOP } moduleWrap_ = std::make_unique( - store_.get(), wasmCode, instantiate, imports, j_); + store_, wasmCode, instantiate, imports, j_); if (!moduleWrap_) throw std::runtime_error( @@ -697,7 +671,7 @@ WasmiEngine::call(FuncInfo const& f, std::vector& in) auto const start = usecs(); #endif - wasm_trap_t* trap = wasm_func_call(f.first, &inv, &ret.r); + wasm_trap_t* trap = wasm_func_call(f.first, &inv, &ret.r.vec_); #ifdef SHOW_CALL_TIME auto const finish = usecs(); @@ -849,16 +823,16 @@ WasmiEngine::runHlp( if (res.f) throw std::runtime_error("<" + std::string(funcName) + "> failure"); - else if (!res.r.size) + else if (!res.r.vec_.size) throw std::runtime_error( "<" + std::string(funcName) + "> return nothing"); // LCOV_EXCL_LINE - assert(res.r.data[0].kind == WASM_I32); + assert(res.r.vec_.data[0].kind == WASM_I32); if (gas == -1) gas = std::numeric_limits::max(); WasmResult const ret{ - res.r.data[0].of.i32, gas - moduleWrap_->getGas()}; + res.r.vec_.data[0].of.i32, gas - moduleWrap_->getGas()}; // #ifdef DEBUG_OUTPUT // auto& j = std::cerr; @@ -957,11 +931,11 @@ WasmiEngine::allocate(int32_t sz) { auto res = call<1>(W_ALLOC, static_cast(sz)); - if (res.f || !res.r.size || (res.r.data[0].kind != WASM_I32) || - !res.r.data[0].of.i32) + if (res.f || !res.r.vec_.size || (res.r.vec_.data[0].kind != WASM_I32) || + !res.r.vec_.data[0].of.i32) throw std::runtime_error( "can't allocate memory, " + std::to_string(sz) + " bytes"); - return res.r.data[0].of.i32; + return res.r.vec_.data[0].of.i32; } wasm_trap_t* @@ -973,7 +947,12 @@ WasmiEngine::newTrap(std::string const& txt) if (!txt.empty()) wasm_name_new(&msg, txt.size() + 1, txt.c_str()); // include 0 - return wasm_trap_new(store_.get(), &msg); + wasm_trap_t* trap = wasm_trap_new(store_.get(), &msg); + + if (!txt.empty()) + wasm_byte_vec_delete(&msg); + + return trap; } // LCOV_EXCL_START