[PM-17733] - [Privilege Escalation] - Unauthorised access allows limited access user to change password of Items (#5452)

* prevent view-only users from updating passwords

* revert change to licensing service

* add tests

* check if organizationId is there

* move logic to private method

* move logic to private method

* move logic into method

* revert change to licensing service

* throw exception when cipher key is created by hidden password users

* fix tests

* don't allow totp or passkeys changes from hidden password users

* add tests

* revert change to licensing service

src/Core/Vault/Services/Implementations/CipherService.cs

@@ -13,7 +13,9 @@ using Bit.Core.Tools.Models.Business;
 using Bit.Core.Tools.Services;
 using Bit.Core.Utilities;
 using Bit.Core.Vault.Entities;
+using Bit.Core.Vault.Enums;
 using Bit.Core.Vault.Models.Data;
+using Bit.Core.Vault.Queries;
 using Bit.Core.Vault.Repositories;
 
 namespace Bit.Core.Vault.Services;
@@ -38,6 +40,7 @@ public class CipherService : ICipherService
     private const long _fileSizeLeeway = 1024L * 1024L; // 1MB
     private readonly IReferenceEventService _referenceEventService;
     private readonly ICurrentContext _currentContext;
+    private readonly IGetCipherPermissionsForUserQuery _getCipherPermissionsForUserQuery;
 
     public CipherService(
         ICipherRepository cipherRepository,
@@ -54,7 +57,8 @@ public class CipherService : ICipherService
         IPolicyService policyService,
         GlobalSettings globalSettings,
         IReferenceEventService referenceEventService,
-        ICurrentContext currentContext)
+        ICurrentContext currentContext,
+        IGetCipherPermissionsForUserQuery getCipherPermissionsForUserQuery)
     {
         _cipherRepository = cipherRepository;
         _folderRepository = folderRepository;
@@ -71,6 +75,7 @@ public class CipherService : ICipherService
         _globalSettings = globalSettings;
         _referenceEventService = referenceEventService;
         _currentContext = currentContext;
+        _getCipherPermissionsForUserQuery = getCipherPermissionsForUserQuery;
     }
 
     public async Task SaveAsync(Cipher cipher, Guid savingUserId, DateTime? lastKnownRevisionDate,
@@ -161,6 +166,7 @@ public class CipherService : ICipherService
         {
             ValidateCipherLastKnownRevisionDateAsync(cipher, lastKnownRevisionDate);
             cipher.RevisionDate = DateTime.UtcNow;
+            await ValidateViewPasswordUserAsync(cipher);
             await _cipherRepository.ReplaceAsync(cipher);
             await _eventService.LogCipherEventAsync(cipher, Bit.Core.Enums.EventType.Cipher_Updated);
 
@@ -966,4 +972,32 @@ public class CipherService : ICipherService
 
         ValidateCipherLastKnownRevisionDateAsync(cipher, lastKnownRevisionDate);
     }
+
+    private async Task ValidateViewPasswordUserAsync(Cipher cipher)
+    {
+        if (cipher.Type != CipherType.Login || cipher.Data == null || !cipher.OrganizationId.HasValue)
+        {
+            return;
+        }
+        var existingCipher = await _cipherRepository.GetByIdAsync(cipher.Id);
+        if (existingCipher == null) return;
+
+        var cipherPermissions = await _getCipherPermissionsForUserQuery.GetByOrganization(cipher.OrganizationId.Value);
+        // Check if user is a "hidden password" user
+        if (!cipherPermissions.TryGetValue(cipher.Id, out var permission) || !(permission.ViewPassword && permission.Edit))
+        {
+            // "hidden password" users may not add cipher key encryption
+            if (existingCipher.Key == null && cipher.Key != null)
+            {
+                throw new BadRequestException("You do not have permission to add cipher key encryption.");
+            }
+            // "hidden password" users may not change passwords, TOTP codes, or passkeys, so we need to set them back to the original values
+            var existingCipherData = JsonSerializer.Deserialize<CipherLoginData>(existingCipher.Data);
+            var newCipherData = JsonSerializer.Deserialize<CipherLoginData>(cipher.Data);
+            newCipherData.Fido2Credentials = existingCipherData.Fido2Credentials;
+            newCipherData.Totp = existingCipherData.Totp;
+            newCipherData.Password = existingCipherData.Password;
+            cipher.Data = JsonSerializer.Serialize(newCipherData);
+        }
+    }
 }

test/Core.Test/Vault/Services/CipherServiceTests.cs

@@ -1,4 +1,5 @@
-using Bit.Core.AdminConsole.Entities;
+using System.Text.Json;
+using Bit.Core.AdminConsole.Entities;
 using Bit.Core.Billing.Enums;
 using Bit.Core.Entities;
 using Bit.Core.Enums;
@@ -9,7 +10,9 @@ using Bit.Core.Services;
 using Bit.Core.Test.AutoFixture.CipherFixtures;
 using Bit.Core.Utilities;
 using Bit.Core.Vault.Entities;
+using Bit.Core.Vault.Enums;
 using Bit.Core.Vault.Models.Data;
+using Bit.Core.Vault.Queries;
 using Bit.Core.Vault.Repositories;
 using Bit.Core.Vault.Services;
 using Bit.Test.Common.AutoFixture;
@@ -797,6 +800,233 @@ public class CipherServiceTests
             Arg.Is<IEnumerable<Cipher>>(arg => !arg.Except(ciphers).Any()));
     }
 
+    private class SaveDetailsAsyncDependencies
+    {
+        public CipherDetails CipherDetails { get; set; }
+        public SutProvider<CipherService> SutProvider { get; set; }
+    }
+
+    private static SaveDetailsAsyncDependencies GetSaveDetailsAsyncDependencies(
+        SutProvider<CipherService> sutProvider,
+        string newPassword,
+        bool viewPassword,
+        bool editPermission,
+        string? key = null,
+        string? totp = null,
+        CipherLoginFido2CredentialData[]? passkeys = null
+        )
+    {
+        var cipherDetails = new CipherDetails
+        {
+            Id = Guid.NewGuid(),
+            OrganizationId = Guid.NewGuid(),
+            Type = CipherType.Login,
+            UserId = Guid.NewGuid(),
+            RevisionDate = DateTime.UtcNow,
+            Key = key,
+        };
+
+        var newLoginData = new CipherLoginData { Username = "user", Password = newPassword, Totp = totp, Fido2Credentials = passkeys };
+        cipherDetails.Data = JsonSerializer.Serialize(newLoginData);
+
+        var existingCipher = new Cipher
+        {
+            Id = cipherDetails.Id,
+            Data = JsonSerializer.Serialize(
+                new CipherLoginData
+                {
+                    Username = "user",
+                    Password = "OriginalPassword",
+                    Totp = "OriginalTotp",
+                    Fido2Credentials = []
+                }
+            ),
+        };
+
+        sutProvider.GetDependency<ICipherRepository>()
+            .GetByIdAsync(cipherDetails.Id)
+            .Returns(existingCipher);
+
+        sutProvider.GetDependency<ICipherRepository>()
+            .ReplaceAsync(Arg.Any<CipherDetails>())
+            .Returns(Task.CompletedTask);
+
+        var permissions = new Dictionary<Guid, OrganizationCipherPermission>
+        {
+            { cipherDetails.Id, new OrganizationCipherPermission { ViewPassword = viewPassword, Edit = editPermission } }
+        };
+
+        sutProvider.GetDependency<IGetCipherPermissionsForUserQuery>()
+            .GetByOrganization(cipherDetails.OrganizationId.Value)
+            .Returns(permissions);
+
+        return new SaveDetailsAsyncDependencies
+        {
+            CipherDetails = cipherDetails,
+            SutProvider = sutProvider,
+        };
+    }
+
+    [Theory, BitAutoData]
+    public async Task SaveDetailsAsync_PasswordNotChangedWithoutViewPasswordPermission(string _, SutProvider<CipherService> sutProvider)
+    {
+        var deps = GetSaveDetailsAsyncDependencies(sutProvider, "NewPassword", viewPassword: false, editPermission: true);
+
+        await deps.SutProvider.Sut.SaveDetailsAsync(
+            deps.CipherDetails,
+            deps.CipherDetails.UserId.Value,
+            deps.CipherDetails.RevisionDate,
+            null,
+            true);
+
+        var updatedLoginData = JsonSerializer.Deserialize<CipherLoginData>(deps.CipherDetails.Data);
+        Assert.Equal("OriginalPassword", updatedLoginData.Password);
+    }
+
+    [Theory, BitAutoData]
+    public async Task SaveDetailsAsync_PasswordNotChangedWithoutEditPermission(string _, SutProvider<CipherService> sutProvider)
+    {
+        var deps = GetSaveDetailsAsyncDependencies(sutProvider, "NewPassword", viewPassword: true, editPermission: false);
+
+        await deps.SutProvider.Sut.SaveDetailsAsync(
+            deps.CipherDetails,
+            deps.CipherDetails.UserId.Value,
+            deps.CipherDetails.RevisionDate,
+            null,
+            true);
+
+        var updatedLoginData = JsonSerializer.Deserialize<CipherLoginData>(deps.CipherDetails.Data);
+        Assert.Equal("OriginalPassword", updatedLoginData.Password);
+    }
+
+    [Theory, BitAutoData]
+    public async Task SaveDetailsAsync_PasswordChangedWithPermission(string _, SutProvider<CipherService> sutProvider)
+    {
+        var deps = GetSaveDetailsAsyncDependencies(sutProvider, "NewPassword", viewPassword: true, editPermission: true);
+
+        await deps.SutProvider.Sut.SaveDetailsAsync(
+            deps.CipherDetails,
+            deps.CipherDetails.UserId.Value,
+            deps.CipherDetails.RevisionDate,
+            null,
+            true);
+
+        var updatedLoginData = JsonSerializer.Deserialize<CipherLoginData>(deps.CipherDetails.Data);
+        Assert.Equal("NewPassword", updatedLoginData.Password);
+    }
+
+    [Theory, BitAutoData]
+    public async Task SaveDetailsAsync_CipherKeyChangedWithPermission(string _, SutProvider<CipherService> sutProvider)
+    {
+        var deps = GetSaveDetailsAsyncDependencies(sutProvider, "NewPassword", viewPassword: true, editPermission: true, "NewKey");
+
+        await deps.SutProvider.Sut.SaveDetailsAsync(
+            deps.CipherDetails,
+            deps.CipherDetails.UserId.Value,
+            deps.CipherDetails.RevisionDate,
+            null,
+            true);
+
+        Assert.Equal("NewKey", deps.CipherDetails.Key);
+    }
+
+    [Theory, BitAutoData]
+    public async Task SaveDetailsAsync_CipherKeyChangedWithoutPermission(string _, SutProvider<CipherService> sutProvider)
+    {
+        var deps = GetSaveDetailsAsyncDependencies(sutProvider, "NewPassword", viewPassword: true, editPermission: false, "NewKey");
+
+        var exception = await Assert.ThrowsAsync<BadRequestException>(() => deps.SutProvider.Sut.SaveDetailsAsync(
+            deps.CipherDetails,
+            deps.CipherDetails.UserId.Value,
+            deps.CipherDetails.RevisionDate,
+            null,
+            true));
+
+        Assert.Contains("do not have permission", exception.Message);
+    }
+
+    [Theory, BitAutoData]
+    public async Task SaveDetailsAsync_TotpChangedWithoutPermission(string _, SutProvider<CipherService> sutProvider)
+    {
+        var deps = GetSaveDetailsAsyncDependencies(sutProvider, "NewPassword", viewPassword: true, editPermission: false, totp: "NewTotp");
+
+        await deps.SutProvider.Sut.SaveDetailsAsync(
+            deps.CipherDetails,
+            deps.CipherDetails.UserId.Value,
+            deps.CipherDetails.RevisionDate,
+            null,
+            true);
+
+        var updatedLoginData = JsonSerializer.Deserialize<CipherLoginData>(deps.CipherDetails.Data);
+        Assert.Equal("OriginalTotp", updatedLoginData.Totp);
+    }
+
+    [Theory, BitAutoData]
+    public async Task SaveDetailsAsync_TotpChangedWithPermission(string _, SutProvider<CipherService> sutProvider)
+    {
+        var deps = GetSaveDetailsAsyncDependencies(sutProvider, "NewPassword", viewPassword: true, editPermission: true, totp: "NewTotp");
+
+        await deps.SutProvider.Sut.SaveDetailsAsync(
+            deps.CipherDetails,
+            deps.CipherDetails.UserId.Value,
+            deps.CipherDetails.RevisionDate,
+            null,
+            true);
+
+        var updatedLoginData = JsonSerializer.Deserialize<CipherLoginData>(deps.CipherDetails.Data);
+        Assert.Equal("NewTotp", updatedLoginData.Totp);
+    }
+
+    [Theory, BitAutoData]
+    public async Task SaveDetailsAsync_Fido2CredentialsChangedWithoutPermission(string _, SutProvider<CipherService> sutProvider)
+    {
+        var passkeys = new[]
+        {
+            new CipherLoginFido2CredentialData
+            {
+                CredentialId = "CredentialId",
+                UserHandle = "UserHandle",
+            }
+        };
+
+        var deps = GetSaveDetailsAsyncDependencies(sutProvider, "NewPassword", viewPassword: true, editPermission: false, passkeys: passkeys);
+
+        await deps.SutProvider.Sut.SaveDetailsAsync(
+            deps.CipherDetails,
+            deps.CipherDetails.UserId.Value,
+            deps.CipherDetails.RevisionDate,
+            null,
+            true);
+
+        var updatedLoginData = JsonSerializer.Deserialize<CipherLoginData>(deps.CipherDetails.Data);
+        Assert.Empty(updatedLoginData.Fido2Credentials);
+    }
+
+    [Theory, BitAutoData]
+    public async Task SaveDetailsAsync_Fido2CredentialsChangedWithPermission(string _, SutProvider<CipherService> sutProvider)
+    {
+        var passkeys = new[]
+        {
+            new CipherLoginFido2CredentialData
+            {
+                CredentialId = "CredentialId",
+                UserHandle = "UserHandle",
+            }
+        };
+
+        var deps = GetSaveDetailsAsyncDependencies(sutProvider, "NewPassword", viewPassword: true, editPermission: true, passkeys: passkeys);
+
+        await deps.SutProvider.Sut.SaveDetailsAsync(
+            deps.CipherDetails,
+            deps.CipherDetails.UserId.Value,
+            deps.CipherDetails.RevisionDate,
+            null,
+            true);
+
+        var updatedLoginData = JsonSerializer.Deserialize<CipherLoginData>(deps.CipherDetails.Data);
+        Assert.Equal(passkeys.Length, updatedLoginData.Fido2Credentials.Length);
+    }
+
     [Theory]
     [BitAutoData]
     public async Task DeleteAsync_WithPersonalCipherOwner_DeletesCipher(