From 331e6f0b2b0dcaa9afba78c97955f5e666454e45 Mon Sep 17 00:00:00 2001 From: Sy Brand Date: Thu, 30 Apr 2026 09:50:42 +0100 Subject: [PATCH 1/3] Protect against GC from validate-bytes --- runtime/fastly/builtins/kv-store.cpp | 17 +++++++++++------ runtime/fastly/builtins/secret-store.cpp | 11 ++++++----- 2 files changed, 17 insertions(+), 11 deletions(-) diff --git a/runtime/fastly/builtins/kv-store.cpp b/runtime/fastly/builtins/kv-store.cpp index aae08f479b..8ace566aa9 100644 --- a/runtime/fastly/builtins/kv-store.cpp +++ b/runtime/fastly/builtins/kv-store.cpp @@ -444,6 +444,7 @@ bool KVStore::put(JSContext *cx, unsigned argc, JS::Value *vp) { std::optional ttl = std::nullopt; std::optional if_gen = std::nullopt; std::optional> metadata = std::nullopt; + std::optional>> metadata_buf; std::optional mode = std::nullopt; if (args.get(2).isObject()) { JS::RootedObject opts_val(cx, &args.get(2).toObject()); @@ -556,15 +557,17 @@ bool KVStore::put(JSContext *cx, unsigned argc, JS::Value *vp) { metadata = std::make_tuple(reinterpret_cast(metadata_str.ptr.get()), metadata_str.len); } else { - auto maybe_byte_data = validate_bytes(cx, metadata_val, "KVStore.put metadata"); - if (!maybe_byte_data) { + metadata_buf = validate_bytes(cx, metadata_val, "KVStore.put metadata"); + if (!metadata_buf) { return ReturnPromiseRejectedWithPendingError(cx, args); } - metadata = maybe_byte_data; + auto& [data, len, noGC] = *metadata_buf; + metadata = std::make_tuple(data, len); } } auto res = kv_store(self).insert(key_chars, body, mode, std::nullopt, metadata, ttl); + if (metadata_buf) std::get<2>(*metadata_buf).reset(); // allow GC after hostcall if (auto *err = res.to_err()) { HANDLE_ERROR(cx, *err); return ReturnPromiseRejectedWithPendingError(cx, args); @@ -622,15 +625,17 @@ bool KVStore::put(JSContext *cx, unsigned argc, JS::Value *vp) { metadata = std::make_tuple(reinterpret_cast(metadata_str.ptr.get()), metadata_str.len); } else { - auto maybe_byte_data = validate_bytes(cx, metadata_val, "KVStore.put metadata"); - if (!maybe_byte_data) { + metadata_buf = validate_bytes(cx, metadata_val, "KVStore.put metadata"); + if (!metadata_buf) { return ReturnPromiseRejectedWithPendingError(cx, args); } - metadata = maybe_byte_data; + auto& [data, len, noGC] = *metadata_buf; + metadata = std::make_tuple(data, len); } } auto insert_res = kv_store(self).insert(key_chars, body, mode, if_gen, metadata, ttl); + if (metadata_buf) std::get<2>(*metadata_buf).reset(); // allow GC after hostcall if (auto *err = insert_res.to_err()) { // Ensure that we throw an exception for all unexpected host errors. HANDLE_ERROR(cx, *err); diff --git a/runtime/fastly/builtins/secret-store.cpp b/runtime/fastly/builtins/secret-store.cpp index 3a7c69d061..bbafcd03b6 100644 --- a/runtime/fastly/builtins/secret-store.cpp +++ b/runtime/fastly/builtins/secret-store.cpp @@ -187,19 +187,20 @@ bool SecretStore::from_bytes(JSContext *cx, unsigned argc, JS::Value *vp) { auto bytes = args.get(0); + host_api::Result res; + { auto maybe_byte_data = validate_bytes(cx, bytes, "SecretStore.fromBytes"); if (!maybe_byte_data) { return false; } - // important no work is done here before the host call so our buffer doesn't move - const uint8_t *data; - size_t len; - std::tie(data, len) = maybe_byte_data.value(); - auto res = host_api::SecretStore::from_bytes(data, len); + auto& [data, len, noGC] = *maybe_byte_data; + res = host_api::SecretStore::from_bytes(data, len); + noGC.reset(); if (auto *err = res.to_err()) { HANDLE_ERROR(cx, *err); return false; } +} JS::SetReservedSlot(entry, Slots::Handle, JS::Int32Value(res.unwrap().handle)); args.rval().setObject(*entry); From f751b892a9ef2f52d031a6df241a84f111a26d40 Mon Sep 17 00:00:00 2001 From: Sy Brand Date: Thu, 30 Apr 2026 09:53:08 +0100 Subject: [PATCH 2/3] Cleanup --- runtime/fastly/builtins/kv-store.cpp | 13 ++++++++----- runtime/fastly/builtins/secret-store.cpp | 7 ++----- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/runtime/fastly/builtins/kv-store.cpp b/runtime/fastly/builtins/kv-store.cpp index 8ace566aa9..bc81c2cc8f 100644 --- a/runtime/fastly/builtins/kv-store.cpp +++ b/runtime/fastly/builtins/kv-store.cpp @@ -444,7 +444,8 @@ bool KVStore::put(JSContext *cx, unsigned argc, JS::Value *vp) { std::optional ttl = std::nullopt; std::optional if_gen = std::nullopt; std::optional> metadata = std::nullopt; - std::optional>> metadata_buf; + std::optional>> + metadata_buf; std::optional mode = std::nullopt; if (args.get(2).isObject()) { JS::RootedObject opts_val(cx, &args.get(2).toObject()); @@ -561,13 +562,14 @@ bool KVStore::put(JSContext *cx, unsigned argc, JS::Value *vp) { if (!metadata_buf) { return ReturnPromiseRejectedWithPendingError(cx, args); } - auto& [data, len, noGC] = *metadata_buf; + auto &[data, len, noGC] = *metadata_buf; metadata = std::make_tuple(data, len); } } auto res = kv_store(self).insert(key_chars, body, mode, std::nullopt, metadata, ttl); - if (metadata_buf) std::get<2>(*metadata_buf).reset(); // allow GC after hostcall + if (metadata_buf) + std::get<2>(*metadata_buf).reset(); // allow GC after hostcall if (auto *err = res.to_err()) { HANDLE_ERROR(cx, *err); return ReturnPromiseRejectedWithPendingError(cx, args); @@ -629,13 +631,14 @@ bool KVStore::put(JSContext *cx, unsigned argc, JS::Value *vp) { if (!metadata_buf) { return ReturnPromiseRejectedWithPendingError(cx, args); } - auto& [data, len, noGC] = *metadata_buf; + auto &[data, len, noGC] = *metadata_buf; metadata = std::make_tuple(data, len); } } auto insert_res = kv_store(self).insert(key_chars, body, mode, if_gen, metadata, ttl); - if (metadata_buf) std::get<2>(*metadata_buf).reset(); // allow GC after hostcall + if (metadata_buf) + std::get<2>(*metadata_buf).reset(); // allow GC after hostcall if (auto *err = insert_res.to_err()) { // Ensure that we throw an exception for all unexpected host errors. HANDLE_ERROR(cx, *err); diff --git a/runtime/fastly/builtins/secret-store.cpp b/runtime/fastly/builtins/secret-store.cpp index bbafcd03b6..6c043009a0 100644 --- a/runtime/fastly/builtins/secret-store.cpp +++ b/runtime/fastly/builtins/secret-store.cpp @@ -187,20 +187,17 @@ bool SecretStore::from_bytes(JSContext *cx, unsigned argc, JS::Value *vp) { auto bytes = args.get(0); - host_api::Result res; - { auto maybe_byte_data = validate_bytes(cx, bytes, "SecretStore.fromBytes"); if (!maybe_byte_data) { return false; } - auto& [data, len, noGC] = *maybe_byte_data; - res = host_api::SecretStore::from_bytes(data, len); + auto &[data, len, noGC] = *maybe_byte_data; + auto res = host_api::SecretStore::from_bytes(data, len); noGC.reset(); if (auto *err = res.to_err()) { HANDLE_ERROR(cx, *err); return false; } -} JS::SetReservedSlot(entry, Slots::Handle, JS::Int32Value(res.unwrap().handle)); args.rval().setObject(*entry); From 19f6e75c34e72293ead20ef255dbac5ddcf23727 Mon Sep 17 00:00:00 2001 From: Sy Brand Date: Thu, 30 Apr 2026 09:53:45 +0100 Subject: [PATCH 3/3] Add files --- runtime/fastly/common/validations.cpp | 21 +++++++++------------ runtime/fastly/common/validations.h | 5 ++++- 2 files changed, 13 insertions(+), 13 deletions(-) diff --git a/runtime/fastly/common/validations.cpp b/runtime/fastly/common/validations.cpp index c191756ed1..b3117196a8 100644 --- a/runtime/fastly/common/validations.cpp +++ b/runtime/fastly/common/validations.cpp @@ -3,6 +3,7 @@ #include "../builtins/fastly.h" #include "../host-api/host_api_fastly.h" #include "encode.h" +#include "mozilla/Maybe.h" using fastly::FastlyGetErrorMessage; @@ -33,7 +34,7 @@ std::optional parse_and_validate_timeout(JSContext *cx, JS::HandleValu return std::round(native_value); } -std::optional> +std::optional>> validate_bytes(JSContext *cx, JS::HandleValue bytes, const char *subsystem) { if (!bytes.isObject()) { JS_ReportErrorNumberASCII(cx, FastlyGetErrorMessage, nullptr, JSMSG_INVALID_BUFFER, subsystem); @@ -47,24 +48,20 @@ validate_bytes(JSContext *cx, JS::HandleValue bytes, const char *subsystem) { return std::nullopt; } - mozilla::Maybe maybeNoGC; - uint8_t *buf; + const uint8_t *buf; size_t length; + bool is_shared; + mozilla::Maybe noGC; if (JS_IsArrayBufferViewObject(bytes_obj)) { - JS::AutoCheckCannotGC noGC; - bool is_shared; + noGC.emplace(); length = JS_GetArrayBufferViewByteLength(bytes_obj); - buf = (uint8_t *)JS_GetArrayBufferViewData(bytes_obj, &is_shared, noGC); + buf = (const uint8_t *)JS_GetArrayBufferViewData(bytes_obj, &is_shared, *noGC); MOZ_ASSERT(!is_shared); - } else if (JS::IsArrayBufferObject(bytes_obj)) { - bool is_shared; - JS::GetArrayBufferLengthAndData(bytes_obj, &length, &is_shared, (uint8_t **)&buf); } else { - JS_ReportErrorNumberASCII(cx, FastlyGetErrorMessage, nullptr, JSMSG_INVALID_BUFFER, subsystem); - return std::nullopt; + JS::GetArrayBufferLengthAndData(bytes_obj, &length, &is_shared, (uint8_t **)&buf); } - return std::make_tuple(buf, length); + return std::make_tuple(buf, length, std::move(noGC)); } } // namespace fastly::common diff --git a/runtime/fastly/common/validations.h b/runtime/fastly/common/validations.h index 251b99c816..bdd9997ece 100644 --- a/runtime/fastly/common/validations.h +++ b/runtime/fastly/common/validations.h @@ -2,7 +2,10 @@ #define FASTLY_VALIDATIONS_H #include "builtin.h" +#include "js/GCAPI.h" +#include "mozilla/Maybe.h" #include +#include namespace fastly::common { @@ -10,7 +13,7 @@ std::optional parse_and_validate_timeout(JSContext *cx, JS::HandleValu const char *subsystem, std::string property_name, uint64_t max_timeout); -std::optional> +std::optional>> validate_bytes(JSContext *cx, JS::HandleValue bytes, const char *subsystem); } // namespace fastly::common