Skip to content

Commit

Permalink
Correctly handle legacy security rejections
Browse files Browse the repository at this point in the history
The code comment of this code was entirely incorrect, but the commit
message for 5671072 when it was added was correct. I.e. there is a
result, but not a reason.

Adjust the unit tests to make sure this doesn't regress again.
  • Loading branch information
CendioOssman committed Sep 7, 2023
1 parent 72f6810 commit 370f21b
Show file tree
Hide file tree
Showing 2 changed files with 20 additions and 14 deletions.
7 changes: 0 additions & 7 deletions core/rfb.js
Original file line number Diff line number Diff line change
Expand Up @@ -1966,13 +1966,6 @@ export default class RFB extends EventTargetMixin {
}

_handleSecurityResult() {
// There is no security choice, and hence no security result
// until RFB 3.7
if (this._rfbVersion < 3.7) {
this._rfbInitState = 'ClientInitialisation';
return true;
}

if (this._sock.rQwait('VNC auth response ', 4)) { return false; }

const status = this._sock.rQshift32();
Expand Down
27 changes: 20 additions & 7 deletions tests/test.rfb.js
Original file line number Diff line number Diff line change
Expand Up @@ -2238,23 +2238,36 @@ describe('Remote Frame Buffer Protocol Client', function () {
});

describe('Legacy SecurityResult', function () {
beforeEach(function () {
it('should not include reason in securityfailure event for versions < 3.7', function () {
client.addEventListener("credentialsrequired", () => {
client.sendCredentials({ password: 'passwd' });
});
sendVer('003.007\n', client);
client._sock._websocket._getSentData();
sendSecurity(2, client);
const spy = sinon.spy();
client.addEventListener("securityfailure", spy);
sendVer('003.006\n', client);
client._sock._websocket._receiveData(new Uint8Array([0, 0, 0, 2]));
const challenge = [];
for (let i = 0; i < 16; i++) { challenge[i] = i; }
client._sock._websocket._receiveData(new Uint8Array(challenge));
client._sock._websocket._getSentData();
clock.tick();

client._sock._websocket._receiveData(new Uint8Array([0, 0, 0, 2]));
expect(spy).to.have.been.calledOnce;
expect(spy.args[0][0].detail.status).to.equal(2);
expect('reason' in spy.args[0][0].detail).to.be.false;
});

it('should not include reason in securityfailure event', function () {
it('should not include reason in securityfailure event for versions < 3.8', function () {
client.addEventListener("credentialsrequired", () => {
client.sendCredentials({ password: 'passwd' });
});
const spy = sinon.spy();
client.addEventListener("securityfailure", spy);
sendVer('003.007\n', client);
sendSecurity(2, client);
const challenge = [];
for (let i = 0; i < 16; i++) { challenge[i] = i; }
client._sock._websocket._receiveData(new Uint8Array(challenge));

client._sock._websocket._receiveData(new Uint8Array([0, 0, 0, 2]));
expect(spy).to.have.been.calledOnce;
expect(spy.args[0][0].detail.status).to.equal(2);
Expand Down

0 comments on commit 370f21b

Please sign in to comment.