Skip to content

Commit bbc9779

Browse files
committed
fix(security): harden untrusted deserialization paths
1 parent 8728bd8 commit bbc9779

153 files changed

Lines changed: 9843 additions & 1342 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

.agents/languages/dart.md

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -95,9 +95,9 @@ Load this file when changing `dart/`.
9595
formulas.
9696
Treat the option as an approximate collection/map/array/struct/object gate, not an exact heap
9797
cap. Leaf values skipped by graph budgeting remain gated by unread input bytes.
98-
- Do not add parallel header-low/header-high slot caches or multi-slot recent caches in TypeMeta hot paths to chase benchmark gaps. Header-cache hits must use the concrete checked cache owner directly; if a hit hint is needed, cache one TypeInfo/TypeMeta object and compare the validated header identity on that object, not separate low/high header fields or benchmark-pattern state.
99-
- If Dart TypeMeta cache ownership changes, keep the invariant in a source comment near the hit path: a checked metadata-cache hit skips the body and must not grow low-bit sentinels, accepted-header fields, parallel header slots, or benchmark-pattern state.
100-
- Dart expected-type TypeDef reads should compare the expected `TypeInfo` object's cached local TypeDef header before consulting the parsed-metadata map. A match is a direct local-schema hit: skip the remote body, add the expected type to the per-read shared type table, and do not publish to `ParsedTypeMetaCache`, record a remote schema version, or parse/hash the body.
98+
- Do not add parallel header-low/header-high slot caches or multi-slot recent caches in TypeMeta hot paths to chase benchmark gaps. Header-cache hits must use the concrete checked cache owner directly; if a hit hint is needed, cache one TypeInfo/TypeMeta object and compare the protocol-defined top 52 header bits on that object, not separate low/high header fields or benchmark-pattern state.
99+
- The top 52 TypeDef/TypeMeta header bits are the schema identity. The full low 12 bits belong only to the current frame and must not participate in hit selection. On a hit, decode the current body size from its low eight bits and any extended-size varuint, prove those bytes readable, and skip exactly that body. Do not validate reserved/compress flags, compare cached or local low bits, parse or rehash the body, repeat schema or policy validation, or grow low-bit sentinels, accepted-header fields, parallel header slots, or benchmark-pattern state. The cold miss path owns low-flag validation.
100+
- Dart expected-type TypeDef reads should compare only the top 52 bits of the expected `TypeInfo` object's cached local TypeDef header before consulting the parsed-metadata map. A match is a direct local-schema hit: use the current frame's size encoding only for bounds and skip, add the expected type to the per-read shared type table, and do not validate its low flags, publish to `ParsedTypeMetaCache`, record a remote schema version, or parse/hash the body.
101101
- Dart local TypeDef construction is registration-owned: record registrations
102102
and finalize their dependent TypeDefs and struct serializers before the first
103103
root read or write. The first `serialize`, `serializeTo`, `serializeBuiltin`,

.agents/languages/rust.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ Load this file when changing `rust/` or Rust xlang behavior.
77
- Run all cargo commands from within `rust/`.
88
- Changes under `rust/` must pass `clippy` and tests.
99
- Rust code must compile without compiler or Clippy warnings. Treat warnings as blockers and keep `cargo clippy --all-targets --all-features -- -D warnings` passing.
10+
- `check_string_read(false)` is a trusted-input-only performance option. It deliberately skips UTF-8 validation, so callers that enable it own the guarantee that serialized string bytes are valid UTF-8. Keep the default checked mode for untrusted input, and do not treat misuse of the unchecked option as a default-path deserialization vulnerability.
1011
- Use `RUST_BACKTRACE=1 FORY_PANIC_ON_ERROR=1` when debugging failing Rust tests.
1112
- Add `-- --nocapture` when you need test output during debugging.
1213
- Do not set `FORY_PANIC_ON_ERROR=1` when running the full Rust test suite, because some tests assert on error contents.

.agents/repo-reference.md

Lines changed: 15 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -58,21 +58,21 @@ Apache Fory is a multi-language serialization framework with multiple wire forma
5858
versions; compatible named enum/ext/union metadata normally has one version but still counts
5959
against remote metadata total limits when it is sent as shared metadata. Pure id-based enum, ext,
6060
and typed-union values use type id plus user type id and must not be moved onto this metadata
61-
cache path. Exact-local metadata bypass is not struct-only: after the existing type and
62-
deserialization-policy checks for the selected local type, compare the original received encoded
63-
bytes with the local encoded metadata bytes, and allow exact matches for struct and named
64-
enum/ext/union metadata without consuming remote schema-version limits. Derive the local
65-
exact-match candidate inside the metadata owner from the decoded `userTypeId` or
66-
`(namespace, typeName)` identity; do not add caller-threaded expected-type parameters only for
67-
this check in any runtime. Java and Python may lazy-build local metadata bytes only after this
68-
identity lookup selects a local class and the existing class, registration, and deserialization
69-
policy checks have run. When a typed read path already has a declared local type and validates the
70-
decoded metadata identity against it, use that same local type for exact-local byte comparison and
71-
cache publish; dynamic or Any paths resolve the local type from decoded metadata. A runtime may
72-
also skip a received metadata body when the current declared type already owns an identical local
73-
metadata header; that is a local-schema hit, not a remote cache publish, and it must not consume
74-
schema-version limits. Other header-only skips are allowed only after the same owning remote
75-
metadata cache has validated a previous body for that header.
61+
cache path. The protocol-defined 52-bit TypeDef/TypeMeta header hash is the unique schema
62+
identity. When the selected local type already owns the received header, that is a local-schema
63+
hit: skip the body and use the local metadata without body comparison, cache publication, or
64+
schema-version counting. A checked remote-cache hit likewise skips the body without rehashing,
65+
byte comparison, repeated validation, or policy work. The low 12 bits describe only the current
66+
frame; a hit uses its current size for bounds and skip without validating reserved or compression
67+
flags. A cache miss is the only path that parses and validates a body. After that first
68+
validation, a runtime may compare the received 52-bit hash with lazily built local metadata when
69+
no local header was available before the parse; hash equality selects the local owner and may
70+
bypass remote schema-version counting without a byte or field comparison.
71+
Derive a miss-only local candidate inside the metadata owner from the decoded identity, after
72+
existing class, registration, and policy checks. A statically declared reader may pass its
73+
concrete expected owner so reference and cache hits can route by owner identity before publish;
74+
do not thread expected-type parameters solely to repeat miss-time metadata validation. Do not add
75+
parallel accepted-header state or retain metadata bytes to revalidate either path.
7676
- Remote metadata body and struct field-count limits are also cold-path resource controls.
7777
`maxTypeMetaBytes` limits one received TypeDef or TypeMeta body excluding the 8-byte header and
7878
extended-size varint; `maxTypeFields` limits one received struct metadata body's field count

AGENTS.md

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,8 @@ This is the entry point for AI guidance in Apache Fory. Read this file first, th
3333
- Do not change an existing `RefReader`/`RefWriter` architecture or API to support compatible skip. Compatible skip must not add alternate reference slots or tables, alternate reference lookup or publication methods, or forwarding APIs in read/write contexts, builders, serializers, or generated-code plumbing. Keep ordinary reference publication and lookup unchanged and resolve the case in the existing compatible generated owner. For an authorized removed-field read of an unregistered Struct, the empty object created by the skip reader is that path's final owner: publish that same object for `RefValue`, consume the Struct fields, and let later `RefFlag` values resolve to it. This preserves reference numbering and identity without registering the Struct; an independent dynamic root still requires normal registration. Do not add parallel reference state, a sentinel, a rejection, or a common-path branch for this case.
3434
- Respect ownership. Keep logic, state, and helpers in their natural owner, and do not move serializer-local, context-local, runtime-type-local, or protocol-local problems into global utilities.
3535
- Check the spec before implementation. For wire behavior and xlang mapping, use the specs as the source of truth and never copy one runtime's bug into another runtime just to make tests pass.
36+
- `foryc` is a build-time compiler for trusted schema inputs and is never invoked by runtime serialization or deserialization. Schema provenance, package/namespace options, output-path options, and generated-source review belong to the application or build owner. Do not classify hostile-schema source injection or path traversal as a Fory runtime security vulnerability; `foryc` does not promise to sandbox untrusted schemas.
37+
- Row format accepts only trusted input and is outside Fory's untrusted binary-deserialization security boundary. Rust `check_string_read(false)` is likewise an explicit trusted-input mode that disables UTF-8 validation; its caller owns the validity guarantee. Classify issues in those paths as correctness, soundness, or hardening bugs when applicable, not as attacker-controlled deserialization vulnerabilities under the default security model.
3638
- Do not make assumptions about runtime behavior, ownership, registration, metadata construction, protocol semantics, or test coverage. Read the current code, owning docs/specs, and relevant tests before making a design judgment or implementation decision. If the evidence is incomplete, inspect more or state the uncertainty explicitly instead of filling gaps from memory or analogy with another runtime.
3739
- For untrusted deserialization, read `docs/security/deserialization.md` before changing allocation, stream filling, skip, reference, metadata, or policy validation behavior. Variable-length deserialization must not allocate or reserve backing/output capacity from attacker-declared lengths or counts before the byte owner has proven proportional readable bytes with `checkReadableBytes` or the runtime equivalent. Root graph memory reservation is accounting only and may happen before that byte check, but it must not replace the byte check.
3840
- Malformed input must surface as a controlled root-operation error and still run
@@ -139,7 +141,8 @@ This is the entry point for AI guidance in Apache Fory. Read this file first, th
139141
or map entry may instead advance because of ref, null, or type envelopes; name those derived
140142
facts `fieldReadAlwaysAdvances`, `elementReadAlwaysAdvances`, or `entryReadAlwaysAdvances` rather
141143
than conflating them with `readData`.
142-
- For remote TypeDef/TypeMeta reads, the checked metadata cache is the only owner of remote "already validated" state. Cache hit means the header was previously parsed, body/hash-validated, policy-checked, and published by that cache, so the hot path must skip the body and use cached metadata without extra validation, hashing, limit checks, exact-local checks, allocation, or policy work. The protocol-defined 52-bit TypeDef/TypeMeta header hash is the unique schema identity, so a known expected local header/hash match is a local-schema hit and must not recompare field arrays or metadata bodies. It may skip the body and use the local TypeInfo/TypeMeta without schema-version counting or cache publish. Cache miss is the only path that parses and validates non-local metadata, enforces limits, and publishes remote metadata to the cache. Do not add nullable accepted-header fields, sentinel headers, per-TypeInfo markers, pending metadata state, parallel header-low/header-high slots, or parallel acceptance state for this decision. If a runtime needs a metadata hit hint, cache the concrete checked metadata owner object, such as the TypeInfo, TypeDef, or TypeMeta used by that runtime, and compare its validated header identity directly.
144+
- For remote TypeDef/TypeMeta reads, the checked metadata cache is the only owner of remote "already validated" state. Cache hit means the header was previously parsed, body/hash-validated, policy-checked, and published by that cache, so the hot path must skip the body and use cached metadata without extra validation, hashing, limit checks, exact-local checks, allocation, or policy work. The protocol-defined 52-bit TypeDef/TypeMeta header hash is the unique schema identity, so a known expected local header/hash match is a local-schema hit and must not recompare field arrays or metadata bodies. The low 12 header bits belong only to the current frame; on a hit, use its current size and optional extension for bounds and skip, but do not validate its reserved or compression flags. A local hit uses the local TypeInfo/TypeMeta without schema-version counting or cache publish. Cache miss is the only path that parses and validates non-local metadata, including low flags, and enforces limits. If the local header becomes available only after that first parse, compare its 52-bit hash with the validated received hash; equality selects the local owner without a second byte or field comparison. Only a non-local miss publishes remote metadata to the cache. Do not add nullable accepted-header fields, sentinel headers, per-TypeInfo markers, pending metadata state, parallel header-low/header-high slots, or parallel acceptance state for this decision. If a runtime needs a metadata hit hint, cache the concrete checked metadata owner object, such as the TypeInfo, TypeDef, or TypeMeta used by that runtime, and compare its validated header identity directly.
145+
- Checked MetaString caches follow the same rule: validate and publish only on cache miss; on cache hit, skip the encoded body and use the cached value without rehashing, comparing body bytes, or repeating validation. The protocol-defined wire hash alone is the MetaString cache identity; the current frame length is used only for bounds checking and advancing the reader, and must not participate in hit selection. Do not add hit-time byte or length comparison or parallel acceptance state for MetaString caches.
143146
- When a user corrects a non-obvious invariant, encode it in the nearest source comment before continuing, and also update `AGENTS.md`, `.agents/**`, docs, or specs when the rule is reusable beyond one file. Do not rely only on chat history, task notes, commit messages, or benchmark logs for corrections that protect security, protocol behavior, ownership, naming, or hot-path performance.
144147
- Reject semantic hacks. Do not bypass broken semantics by deleting cases, simplifying callers, adding coercion hooks, or using workaround fallbacks; fix the underlying bug and prove it with focused tests.
145148
- Protect hot paths. Avoid per-call allocations, callback objects, result tuples or records, unnecessary runtime branches, and wrapper-class substitutions in hot codec/runtime paths; prefer conditional imports and allocation-free concrete implementations where they fit the language.

cpp/fory/serialization/collection_serializer.h

Lines changed: 18 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -100,11 +100,22 @@ template <typename T> inline constexpr bool need_type_for_collection_elem() {
100100
template <typename T>
101101
FORY_ALWAYS_INLINE const TypeInfo *
102102
read_collection_element_type_info(ReadContext &ctx) {
103-
const TypeInfo *type_info = ctx.read_any_type_info(ctx.error());
103+
using ElemType = nullable_element_t<T>;
104+
const TypeInfo *expected_type_info = nullptr;
105+
if constexpr (is_user_type(Serializer<ElemType>::type_id)) {
106+
auto expected_result =
107+
ctx.type_resolver().template get_type_info<ElemType>();
108+
if (FORY_PREDICT_FALSE(!expected_result.ok())) {
109+
ctx.set_error(std::move(expected_result).error());
110+
return nullptr;
111+
}
112+
expected_type_info = expected_result.value();
113+
}
114+
const TypeInfo *type_info =
115+
ctx.read_any_type_info_owner(ctx.error(), expected_type_info);
104116
if (FORY_PREDICT_FALSE(ctx.has_error())) {
105117
return nullptr;
106118
}
107-
using ElemType = nullable_element_t<T>;
108119
constexpr uint32_t expected =
109120
static_cast<uint32_t>(Serializer<ElemType>::type_id);
110121
if (FORY_PREDICT_FALSE(!type_id_matches(type_info->type_id, expected))) {
@@ -796,7 +807,11 @@ inline Container read_collection_data_slow(ReadContext &ctx, uint32_t length) {
796807
const TypeInfo *elem_type_info = nullptr;
797808
Harness::ReadAsFn elem_reader = nullptr;
798809
if (is_same_type && !is_decl_type) {
799-
elem_type_info = ctx.read_any_type_info(ctx.error());
810+
if constexpr (elem_is_polymorphic) {
811+
elem_type_info = ctx.read_any_type_info(ctx.error());
812+
} else {
813+
elem_type_info = read_collection_element_type_info<T>(ctx);
814+
}
800815
if (FORY_PREDICT_FALSE(ctx.has_error())) {
801816
return result;
802817
}

0 commit comments

Comments
 (0)