From 7d2ca68e575f94b96b7a3ab4a85add2a35523433 Mon Sep 17 00:00:00 2001 From: Robbin Janssen Date: Tue, 11 Aug 2026 10:01:01 +0000 Subject: [PATCH] Wipe keys before dispatching decrypt-failure events The Laravel events expose their SecureMessage as a public readonly property, so listeners (and anything they serialise the event to, such as a queued listener writing to Redis) can read it. Most decrypt-failure paths hand over a wiped instance, because the DecryptException constructor wipes the keys when it is given the secure message. Three paths throw without it and left the decrypted key material on the dispatched instance: - a missing storage key file (database key + verification code present); - malformed stored ciphertext (all key parts present); - a missing file blob (database key + verification code present). This violates the split-key promise that the key parts never co-locate. Wipe the secure message in the catch block, before any event is dispatched, regardless of whether the exception carried it. The already-wiped paths are unaffected (wiping is idempotent). Regression tests assert the dispatched event carries no key material on both null paths. Co-Authored-By: Claude Fable 5 --- src/Laravel/Factory.php | 7 +++++++ tests/Laravel/FactoryTest.php | 14 ++++++++++++++ 2 files changed, 21 insertions(+) diff --git a/src/Laravel/Factory.php b/src/Laravel/Factory.php index 22dd73e..28fe09a 100644 --- a/src/Laravel/Factory.php +++ b/src/Laravel/Factory.php @@ -259,6 +259,13 @@ public function decryptMessage(string $secureMessageId, string $verificationCode $record->save(); } + // Wipe the keys before the secure message is handed to event listeners. Most failure paths + // already wipe the keys (the DecryptException constructor does so when it is given the secure + // message), but the paths that throw without it - a missing key file, a missing file blob or + // malformed stored ciphertext - would otherwise expose the decrypted keys on this instance to + // listeners (and to anything they serialize the event to, such as a queue). + $secureMessage->wipeKeysFromMemory(); + // Dispatch events. match ($exception::class) { HitPointLimitReachedException::class => $this->event->dispatch(new HitPointLimitReached($secureMessage)), diff --git a/tests/Laravel/FactoryTest.php b/tests/Laravel/FactoryTest.php index 288d081..3b95fcb 100644 --- a/tests/Laravel/FactoryTest.php +++ b/tests/Laravel/FactoryTest.php @@ -187,6 +187,13 @@ public function testDecryptMessageStorageKeyNotFound(): void $secureMessageFactoryMock->shouldReceive('setMetaKey')->withArgs(['metaKey'])->once()->andReturnSelf(); $eventMock->shouldReceive('dispatch')->withArgs([\Mockery::on(function ($event) { + // The event must not expose any key material to listeners, even though this failure path + // throws without a secure message (so the DecryptException constructor never wiped it). + $this->assertNull($event->secureMessage->getDatabaseKey()); + $this->assertNull($event->secureMessage->getStorageKey()); + $this->assertNull($event->secureMessage->getMetaKey()); + $this->assertNull($event->secureMessage->getVerificationCode()); + return $event::class === DecryptionFailed::class; })])->once(); @@ -489,6 +496,13 @@ public function testDecryptFileMessageBlobMissing(): void $secureMessageFactoryMock->shouldReceive('setMetaKey')->withArgs(['metaKey'])->once()->andReturnSelf(); $eventMock->shouldReceive('dispatch')->withArgs([\Mockery::on(function ($event) { + // The event must not expose any key material to listeners, even though this failure path + // throws without a secure message (so the DecryptException constructor never wiped it). + $this->assertNull($event->secureMessage->getDatabaseKey()); + $this->assertNull($event->secureMessage->getStorageKey()); + $this->assertNull($event->secureMessage->getMetaKey()); + $this->assertNull($event->secureMessage->getVerificationCode()); + return $event::class === DecryptionFailed::class; })])->once();