Skip to content

[Security] HMAC integrity tag is strippable — clearing FLAG_INTEGRITY_HMAC downgrades to no verification because requireIntegrity defaults to false and withIntegrity() does not set it #579

Description

@pathosDev

Component: src/persistence/object-storage/BodyCodec.ts
Severity (assessment): MEDIUM
CWE: CWE-757

decodeBody decides whether to verify the HMAC from a bit in the attacker-controlled manifest byte. An attacker who rewrites the body with bit4 cleared and the 16-byte tag removed takes the else if branch, and since requireIntegrity is false by default that branch is a no-op. The #116 tamper protection is therefore fully bypassable in the configuration a developer gets from calling withIntegrity(...) alone.

Exploit walkthrough

Attacker = the same one #116 was written against: write access to the object-storage backend (co-tenant, compromised sibling service, hostile provider, local process on the FS backend). The deployment has enabled integrity the natural way — ObjectStorageDurableStateStoreOptions.create().withBackend(b).withIntegrity({ mode: 'hmac-sha256', integrityKey }) — and believes bodies are tamper-evident. The attacker fetches <prefix><pid>/state.json, writes back ATS1 + flags byte with FLAG_INTEGRITY_HMAC (0b10000) cleared + arbitrary JSON payload, and drops the 16 trailing tag bytes. On the next load() the store still passes integrity: { integrityKey, requireIntegrity: false }, hasIntegrity is false, no HMAC is computed, and the forged {revision, state} is parsed and cached (ObjectStorageDurableStateStore.ts:121-128). Concretely this is the exact #116 exploit — set "revision":999 and an attacker-chosen state — with one extra byte of work. The forged revision is then cached and used to build the next If-Match, so the write path accepts it too.

Evidence — src/persistence/object-storage/BodyCodec.ts:239

} else if (options.integrity?.requireIntegrity) {
    throw new Error(
      'BodyCodec: body has no integrity tag but requireIntegrity=true was set.  '
      + 'Body was either written before integrity was enabled, or is being injected '
      + 'as part of a downgrade attack.',
    );
  }

// src/persistence/durable-state-stores/ObjectStorageDurableStateStore.ts:80
//   this.requireIntegrity = resolvedOptions.requireIntegrity ?? false;
// ...:105
//               requireIntegrity: this.requireIntegrity,

// src/persistence/durable-state-stores/ObjectStorageDurableStateStoreOptions.ts:94-96
//   withIntegrity(integrity: IntegrityConfig | IntegrityResolver): this {
//     return this.set('integrity', integrity);
//   }            // <- does not touch requireIntegrity

Why the existing guard does not cover it

The guard exists and works: requireIntegrity: true throws on an untagged body, and tests/integration/in-process/persistence/object-storage/IntegrityTampering.test.ts:181 pins it ('requireIntegrity=true rejects a legacy body (downgrade protection)'), with a second test rejecting a requireIntegrity without an integrity config. I also confirmed the positive path is real — a flipped byte and a wrong-key forgery are both rejected, and constantTimeEqual (Integrity.ts:71-76) is a genuine constant-time compare. What it does not cover is the default: requireIntegrity is a separate, second builder call (withRequireIntegrity()), it defaults to false, and the field's own JSDoc frames it as a post-migration nicety ("Legacy bodies without the integrity flag still decode cleanly — tag is opt-in"). So the API's natural single call turns on a control that an attacker disables for free. This is the framework default being unsafe rather than a missing mechanism, which is why I am reporting it separately from #116 (which is fixed for the modify-in-place case).

Suggested fix

Make the safe state the default for new deployments: have withIntegrity({mode:'hmac-sha256', ...}) imply requireIntegrity: true unless the caller explicitly opts out (rename the escape hatch to something like withAllowUntaggedBodies(true) so the legacy-corpus case is the one that has to be spelled out). At minimum, log a startup warning when integrity.mode === 'hmac-sha256' and requireIntegrity is false, and add a regression test that an integrity-configured store with default options rejects a tag-stripped body.

Verification status

Found in the whole-framework security audit of 2026-08-01 (v0.12.0), then adjudicated by an independent verifier instructed to refute it.

Verifier note

Verified line by line. BodyCodec.ts:214 derives hasIntegrity from the attacker-controlled flags byte; BodyCodec.ts:239 is the quoted } else if (options.integrity?.requireIntegrity) { branch, so with the flag cleared and the 16 trailing bytes dropped no HMAC is computed. ObjectStorageDurableStateStore.ts:80 is this.requireIntegrity = resolvedOptions.requireIntegrity ?? false and line 105 forwards that false into the decode options; ObjectStorageDurableStateStoreOptions.ts:94-96 withIntegrity only calls this.set('integrity', …) and never touches requireIntegrity. Grepping requireIntegrity across src/ returns only those sites — there is no HOCON default and nothing in src/config/ sets it. The forged body is then trusted at ObjectStorageDurableStateStore.ts:128, which caches the attacker's revision.

Correction applied: The finding understates the mechanism and overstates the severity. Understated: FLAG_ENCRYPTED is strippable the same way. decodeBody branches on if (encrypted) at BodyCodec.ts:250 and there is no "require encryption" option, so a reader configured with a subkey silently accepts a body whose bit2 is cleared and whose payload is plaintext. That means requireIntegrity=false leaves even a client-aes256-gcm deployment forgeable, not just an unencrypted one; conversely requireIntegrity=true is a complete fix, because the tag covers the flags byte. Also worth noting the one-call wiring cannot enable this at all — ObjectStoragePlugin.ts:115-122 forwards prefix/compression/encryption/maxDecompressedBytes and no integrity field, and ObjectStoragePluginOptions.ts has none. Overstated as high: this is an unsafe-default / opt-in-hardening issue where the mechanism exists, is documented (ObjectStorageDurableStateStoreOptions.ts:38-44) and is pinned by tests, and the attacker must already hold write access to the state bucket. Medium.

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't workingpriority: mediumUseful, not urgentproduction-goalBlocks or defines the path to production readinesssecuritySecurity-relevant — see severity label for impact tierseverity: mediumModerate impact or requires specific conditions

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions