1
0
mirror of https://github.com/bitwarden/browser.git synced 2025-01-02 18:17:46 +01:00

fix length allowing negative values on Chrome extension (#11926)

This commit is contained in:
✨ Audrey ✨ 2024-11-11 10:48:32 -05:00 committed by GitHub
parent 19e786f820
commit 888b9e346c
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
3 changed files with 52 additions and 4 deletions

View File

@ -373,7 +373,11 @@ describe("UserStateSubject", () => {
singleUserId$.next(SomeUser);
await awaitAsync();
expect(state.nextMock).toHaveBeenCalledWith({ foo: "next" });
expect(state.nextMock).toHaveBeenCalledWith({
foo: "next",
// FIXME: don't leak this detail into the test
"$^$ALWAYS_UPDATE_KLUDGE_PROPERTY$^$": 0,
});
});
it("waits to evaluate `UserState.update` until singleUserEncryptor$ emits", async () => {
@ -394,7 +398,13 @@ describe("UserStateSubject", () => {
await awaitAsync();
const encrypted = { foo: "encrypt(next)" };
expect(state.nextMock).toHaveBeenCalledWith({ id: null, secret: encrypted, disclosed: null });
expect(state.nextMock).toHaveBeenCalledWith({
id: null,
secret: encrypted,
disclosed: null,
// FIXME: don't leak this detail into the test
"$^$ALWAYS_UPDATE_KLUDGE_PROPERTY$^$": 0,
});
});
it("applies dynamic constraints", async () => {

View File

@ -43,6 +43,23 @@ import { UserStateSubjectDependencies } from "./user-state-subject-dependencies"
type Constrained<State> = { constraints: Readonly<Constraints<State>>; state: State };
// FIXME: The subject should always repeat the value when it's own `next` method is called.
//
// Chrome StateService only calls `next` when the underlying values changes. When enforcing,
// say, a minimum constraint, any value beneath the minimum becomes the minimum. This prevents
// invalid data received in sequence from calling `next` because the state provider doesn't
// emit.
//
// The hack is pretty simple. Insert arbitrary data into the saved data to ensure
// that it *always* changes.
//
// Any real fix will be fairly complex because it needs to recognize *fast* when it
// is waiting. Alternatively, the kludge could become a format properly fed by random noise.
//
// NOTE: this only matters for plaintext objects; encrypted fields change with every
// update b/c their IVs change.
const ALWAYS_UPDATE_KLUDGE = "$^$ALWAYS_UPDATE_KLUDGE_PROPERTY$^$";
/**
* Adapt a state provider to an rxjs subject.
*
@ -420,8 +437,25 @@ export class UserStateSubject<
private inputSubscription: Unsubscribable;
private outputSubscription: Unsubscribable;
private counter = 0;
private onNext(value: unknown) {
this.state.update(() => value).catch((e: any) => this.onError(e));
this.state
.update(() => {
if (typeof value === "object") {
// related: ALWAYS_UPDATE_KLUDGE FIXME
const counter = this.counter++;
if (counter > Number.MAX_SAFE_INTEGER) {
this.counter = 0;
}
const kludge = value as any;
kludge[ALWAYS_UPDATE_KLUDGE] = counter;
}
return value;
})
.catch((e: any) => this.onError(e));
}
private onError(value: any) {

View File

@ -1163,7 +1163,11 @@ describe("CredentialGeneratorService", () => {
await awaitAsync();
const result = await firstValueFrom(stateProvider.getUserState$(SettingsKey, SomeUser));
expect(result).toEqual({ foo: "next value" });
expect(result).toEqual({
foo: "next value",
// FIXME: don't leak this detail into the test
"$^$ALWAYS_UPDATE_KLUDGE_PROPERTY$^$": 0,
});
});
it("waits for the user to become available", async () => {