keepassxreboot / keepassxc-browser

KeePassXC Browser Extension
GNU General Public License v3.0
1.78k stars 188 forks source link

Illegal Invocation error on getPublicKey and getPublicKeyAlgorithm calls #2322

Closed Keroosha closed 1 month ago

Keroosha commented 3 months ago

Expected Behavior

Do not get Illegal Invocation error on calling getPublicKey and getPublicKeyAlgorithm methods or override them instead of inheriting from prototype

Current Behavior

If you trying to call getPublicKey or getPublicKeyAlgorithm methods from AuthenticatorAttestationResponse of KeepassXC app you will get Illegal invocation due to inheritance those functions via AuthenticatorAttestationResponse prototype

Possible Solution

Fix createAttestationResponse in passkeys.js as:

const createAttestationResponse = function(publicKey) {
    const response = {
        attestationObject: kpxcBase64ToArrayBuffer(publicKey.response.attestationObject),
        clientDataJSON: kpxcBase64ToArrayBuffer(publicKey.response.clientDataJSON),
        getAuthenticatorData: () => kpxcBase64ToArrayBuffer(publicKey.response?.authenticatorData),
        getTransports: () => [ 'internal' ]
    };

    // Prevent Illegal invocation error
    const responseWithProto = Object.setPrototypeOf(response, AuthenticatorAttestationResponse.prototype);
    responseWithProto.getPublicKey = undefined;
    responseWithProto.getPublicKeyAlgorithm = undefined;

    return responseWithProto;
};

this solution proposed via #2323 PR

Steps to Reproduce (for bugs)

  1. Initiate Registration ceremony
  2. Confirm new passkeys creation at KeepassXC interface
  3. Try to call publicKey.response.getPublicKey or publicKey.response.getPublicKeyAlgorithm in userside JS code
  4. get Illegal invocation Exception

Debug info

KeePassXC - 2.7.9 KeePassXC-Browser - 1.9.2 Operating system: Linux Browser: Chromium

droidmonkey commented 3 months ago

For background, why would those function stubs be called in the first place? Should they actually return the data indicated by the name?

Keroosha commented 3 months ago

why would those function stubs be called in the first place?

Because there is no other way to verify from user code should or not you call those functions (if you trying to check response from debugger they appears as callable functions)

https://www.w3.org/TR/webauthn-3/#iface-authenticatorattestationresponse

ArrayBuffer? getPublicKey();

getPublicKey() This operation returns the DER SubjectPublicKeyInfo of the new credential, or null if this is not available. See § 5.2.1.1 Easily accessing credential data.

Keroosha commented 3 months ago

@droidmonkey Ah, I've get what do you mean

Yeah, I didn't found publicKey in KeepassXC response to return it via AuthenticatorAttestationResponse contract