nhyatt/putty-source
1
0
Fork 0
mirror of https://git.tartarus.org/simon/putty.git synced 2025-01-09 17:38:00 +00:00
Code Issues Projects Releases Wiki Activity

Improve time-safety of XDM-AUTHORIZATION-1 validation.

Browse Source 
While writing the previous patch, I realise that walking along a
decrypted string and stopping to complain about the first mismatch you
find is an anti-pattern. If we're going to deliberately give the same
error message for various mismatches, so as not to give away which
part failed first, then we should also avoid giving away the same
information via a timing leak!

I don't think this is serious enough to warrant the full-on advisory
protocol, because XDM-AUTHORIZATION-1 is rarely used these days and
also DES-based, so there are bigger problems with it. (Plus, why on
earth is it based on encryption anyway, not a MAC?) But since I
spotted it in passing, might as well fix it.
This commit is contained in:
Simon Tatham 2023-04-01 16:07:29 +01:00
parent dff4bd4d14
commit 8e7e3c5944
1 changed files with 17 additions and 15 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

32
ssh/x11fwd.c
View File

@ -231,22 +231,24 @@ static char *x11_verify(unsigned long peer_ip, int peer_port,
return dupstr("cannot do XDM-AUTHORIZATION-1 without remote "
"address data");
des_decrypt_xdmauth(auth->data+9, data, 24);
if (memcmp(auth->data, data, 8) != 0) {
/* cookie wrong */
return dupstr("XDM-AUTHORIZATION-1 data failed check");
}
if (GET_32BIT_MSB_FIRST(data+8) != peer_ip) {
/* IP wrong */
return dupstr("XDM-AUTHORIZATION-1 data failed check");
}
if ((int)GET_16BIT_MSB_FIRST(data+12) != peer_port) {
/* port wrong */
return dupstr("XDM-AUTHORIZATION-1 data failed check");
}
t = GET_32BIT_MSB_FIRST(data+14);
/* Bitwise-OR together any mismatches in the fixed parts of
* the data, to allow checking it all at once */
uint32_t mismatches = 0;
/* Check non-key half of auth cookie */
for (i = 0; i < 8; i++)
mismatches |= auth->data[i] ^ data[i];
/* Check IP address and port */
mismatches |= GET_32BIT_MSB_FIRST(data+8) ^ peer_ip;
mismatches |= (unsigned short)(GET_16BIT_MSB_FIRST(data+12) ^
peer_port);
/* Check zero padding */
for (i = 18; i < 24; i++)
if (data[i] != 0) /* zero padding wrong */
return dupstr("XDM-AUTHORIZATION-1 data failed check");
mismatches |= data[i];
if (mismatches)
return dupstr("XDM-AUTHORIZATION-1 data failed check");
t = GET_32BIT_MSB_FIRST(data+14);
tim = time(NULL);
if (((unsigned long)t - (unsigned long)tim
+ XDM_MAXSKEW) > 2*XDM_MAXSKEW)