From 4d3d01f0537e02a1c0f6e132356a0ddbadf3aee3 Mon Sep 17 00:00:00 2001 From: Nik Gilmore Date: Mon, 18 May 2026 15:24:12 -0700 Subject: [PATCH] Fix exceptions when serialzing blob-encrypted ciphers --- .../Vault/Controllers/CiphersController.cs | 2 +- .../Models/Response/CipherResponseModel.cs | 21 +++++---- src/Core/Vault/Entities/Cipher.cs | 11 +++++ .../Controllers/CiphersControllerTests.cs | 43 +++++++++++++++++++ .../Response/CipherResponseModelTests.cs | 39 +++++++++++++++++ test/Core.Test/Vault/Models/CipherTests.cs | 22 ++++++++++ 6 files changed, 129 insertions(+), 9 deletions(-) diff --git a/src/Api/Vault/Controllers/CiphersController.cs b/src/Api/Vault/Controllers/CiphersController.cs index 27cee3175448..6b91ed1b0f15 100644 --- a/src/Api/Vault/Controllers/CiphersController.cs +++ b/src/Api/Vault/Controllers/CiphersController.cs @@ -1645,7 +1645,7 @@ private void ValidateAttachment() private void ValidateClientVersionForFido2CredentialSupport(Cipher cipher) { - if (cipher.Type == Core.Vault.Enums.CipherType.Login) + if (cipher.Type == Core.Vault.Enums.CipherType.Login && !cipher.IsDataBlobEncrypted()) { var loginData = JsonSerializer.Deserialize(cipher.Data); if (loginData?.Fido2Credentials != null && _currentContext.ClientVersion < _fido2KeyCipherMinimumVersion) diff --git a/src/Api/Vault/Models/Response/CipherResponseModel.cs b/src/Api/Vault/Models/Response/CipherResponseModel.cs index 3e97400a2212..808e9e94b921 100644 --- a/src/Api/Vault/Models/Response/CipherResponseModel.cs +++ b/src/Api/Vault/Models/Response/CipherResponseModel.cs @@ -26,6 +26,19 @@ public CipherMiniResponseModel(Cipher cipher, IGlobalSettings globalSettings, bo Id = cipher.Id; Type = cipher.Type; Data = cipher.Data; + RevisionDate = cipher.RevisionDate; + OrganizationId = cipher.OrganizationId; + Attachments = AttachmentResponseModel.FromCipher(cipher, globalSettings); + OrganizationUseTotp = orgUseTotp; + CreationDate = cipher.CreationDate; + DeletedDate = cipher.DeletedDate; + Reprompt = cipher.Reprompt.GetValueOrDefault(CipherRepromptType.None); + Key = cipher.Key; + + if (cipher.IsDataBlobEncrypted()) + { + return; + } CipherData cipherData; switch (cipher.Type) @@ -78,14 +91,6 @@ public CipherMiniResponseModel(Cipher cipher, IGlobalSettings globalSettings, bo Notes = cipherData.Notes; Fields = cipherData.Fields?.Select(f => new CipherFieldModel(f)); PasswordHistory = cipherData.PasswordHistory?.Select(ph => new CipherPasswordHistoryModel(ph)); - RevisionDate = cipher.RevisionDate; - OrganizationId = cipher.OrganizationId; - Attachments = AttachmentResponseModel.FromCipher(cipher, globalSettings); - OrganizationUseTotp = orgUseTotp; - CreationDate = cipher.CreationDate; - DeletedDate = cipher.DeletedDate; - Reprompt = cipher.Reprompt.GetValueOrDefault(CipherRepromptType.None); - Key = cipher.Key; } public Guid Id { get; set; } diff --git a/src/Core/Vault/Entities/Cipher.cs b/src/Core/Vault/Entities/Cipher.cs index 4dec4ace0761..6200f5a7c47d 100644 --- a/src/Core/Vault/Entities/Cipher.cs +++ b/src/Core/Vault/Entities/Cipher.cs @@ -32,6 +32,17 @@ public void SetNewId() Id = CoreHelpers.GenerateComb(); } + public bool IsDataBlobEncrypted() + { + if (string.IsNullOrWhiteSpace(Data)) + { + return false; + } + + var span = Data.AsSpan().TrimStart(); + return span.Length > 0 && span[0] != '{'; + } + public Dictionary GetAttachments() { if (string.IsNullOrWhiteSpace(Attachments)) diff --git a/test/Api.Test/Vault/Controllers/CiphersControllerTests.cs b/test/Api.Test/Vault/Controllers/CiphersControllerTests.cs index 10a5c6427639..849036bec930 100644 --- a/test/Api.Test/Vault/Controllers/CiphersControllerTests.cs +++ b/test/Api.Test/Vault/Controllers/CiphersControllerTests.cs @@ -61,6 +61,49 @@ public async Task PutPartialShouldReturnCipherWithGivenFolderAndFavoriteValues(U Assert.Equal(isFavorite, result.Favorite); } + [Theory, BitAutoData] + public async Task Put_OpaqueLoginCipherWithOldClient_SkipsFido2VersionCheck( + User user, + SutProvider sutProvider) + { + var cipherId = Guid.NewGuid(); + var cipherDetails = new CipherDetails + { + Id = cipherId, + UserId = user.Id, + Type = CipherType.Login, + Data = "2.iv|ct|mac", + Edit = true, + ViewPassword = true, + }; + + sutProvider.GetDependency() + .GetUserByPrincipalAsync(Arg.Any()) + .Returns(user); + sutProvider.GetDependency() + .GetByIdAsync(cipherId, user.Id) + .Returns(cipherDetails); + sutProvider.GetDependency() + .ClientVersion + .Returns(new Version(2022, 1, 0)); + + var model = new CipherRequestModel + { + Type = CipherType.Login, + Name = "2.name|encrypted", + Data = "2.iv|ct|mac", + }; + + var response = await sutProvider.Sut.Put(cipherId, model); + + Assert.NotNull(response); + Assert.Equal("2.iv|ct|mac", response.Data); + Assert.Null(response.Login); + await sutProvider.GetDependency() + .Received(1) + .SaveDetailsAsync(Arg.Any(), user.Id, Arg.Any(), Arg.Any>()); + } + [Theory, BitAutoData] public async Task PutPartialShouldThrowNotFoundExceptionWhenCipherDoesNotExist(User user, Guid folderId, SutProvider sutProvider) { diff --git a/test/Api.Test/Vault/Models/Response/CipherResponseModelTests.cs b/test/Api.Test/Vault/Models/Response/CipherResponseModelTests.cs index 05e42556422c..f0f29ddc5324 100644 --- a/test/Api.Test/Vault/Models/Response/CipherResponseModelTests.cs +++ b/test/Api.Test/Vault/Models/Response/CipherResponseModelTests.cs @@ -269,4 +269,43 @@ public void Constructor_Passport_PreservesRawDataField() Assert.Equal(serializedData, response.Data); } + + [Theory] + [InlineData(CipherType.Login)] + [InlineData(CipherType.SecureNote)] + [InlineData(CipherType.Card)] + [InlineData(CipherType.Identity)] + [InlineData(CipherType.SSHKey)] + [InlineData(CipherType.BankAccount)] + [InlineData(CipherType.DriversLicense)] + [InlineData(CipherType.Passport)] + public void Constructor_OpaqueData_DoesNotThrowAndSkipsLegacyFields(CipherType type) + { + const string opaque = "2.iv|ct|mac"; + var cipher = new Cipher + { + Id = Guid.NewGuid(), + Type = type, + Data = opaque, + RevisionDate = DateTime.UtcNow, + CreationDate = DateTime.UtcNow, + }; + + var response = new CipherMiniResponseModel(cipher, _globalSettings, false); + + Assert.Equal(type, response.Type); + Assert.Equal(opaque, response.Data); + Assert.Null(response.Name); + Assert.Null(response.Notes); + Assert.Null(response.Login); + Assert.Null(response.SecureNote); + Assert.Null(response.Card); + Assert.Null(response.Identity); + Assert.Null(response.SSHKey); + Assert.Null(response.BankAccount); + Assert.Null(response.DriversLicense); + Assert.Null(response.Passport); + Assert.Null(response.Fields); + Assert.Null(response.PasswordHistory); + } } diff --git a/test/Core.Test/Vault/Models/CipherTests.cs b/test/Core.Test/Vault/Models/CipherTests.cs index 6d557825cb6f..053327f98fff 100644 --- a/test/Core.Test/Vault/Models/CipherTests.cs +++ b/test/Core.Test/Vault/Models/CipherTests.cs @@ -23,4 +23,26 @@ public void Clone_OrganizationCipher_CreatesExactCopy(Cipher cipher) { Assert.Equal(JsonSerializer.Serialize(cipher), JsonSerializer.Serialize(cipher.Clone())); } + + [Theory] + [InlineData(null)] + [InlineData("")] + [InlineData(" ")] + [InlineData("{\"Name\":\"x\"}")] + [InlineData(" { \"Name\": \"x\" }")] + public void IsDataBlobEncrypted_LegacyOrEmpty_ReturnsFalse(string? data) + { + var cipher = new Cipher { Data = data! }; + Assert.False(cipher.IsDataBlobEncrypted()); + } + + [Theory] + [InlineData("2.iv|ct|mac")] + [InlineData("plain string")] + [InlineData(" 2.iv|ct|mac")] + public void IsDataBlobEncrypted_OpaqueData_ReturnsTrue(string data) + { + var cipher = new Cipher { Data = data }; + Assert.True(cipher.IsDataBlobEncrypted()); + } }