ord() redefinition in rfb.py causing log-in issues
Closed this issue · 6 comments
We've run into an issue with logging in with vncdotool to take a screenshot of the remote machine. The cli implementation does not send the correct password to the remote machine.
- I do not know the VNC server or version but I don't think it matters for this issue.
- vncdotool version: 1.0.0
- Reproduce: attempt to get screen capture using
vncdo -s [ip address] capture [imgname].png
The root of the issue appears to be the ord() redefinition at lines 28 - 30 in rfb.py. Commenting out that redefinition fixes the login issue for us, but (without testing) I assume this causes issues with the rest of the file (though it does get the screencap we wanted).
I don't really know much about this package but it seems to me that instead of redefining ord() y'all should make a local version with a local name (even if that's just _ord()).
The redefinition the ord() causes it to return the same parameter given to it. So when trying to reverse the key in setKey function (line 836) it tries to compare a string to an integer, which causes a crash. I commented the redefinition also and it fixed my problems when trying to connect with a password.
The redefinition causes for example ord('x') to return 'x' instead of the integer value the setKey function needs.
for ki in range(len(key)):
bsrc = ord(key[ki])
btgt = 0
for i in range(8):
if bsrc & (1 << i): <--- bsrc needs to be an integer here.
btgt = btgt | (1 << 7-i)
newkey.append(chr(btgt))
But as Will stated above, I have no idea if removing the redefinition causes problems elsewhere. I only connected and took a screenshot, because that is enough for me to know that VNC is actually working.
The fix seems to be to comment out lines 30-31 in rfb.py. Change this:
if not isinstance(b' ', str):
def ord(x): return x
to this:
'''if not isinstance(b' ', str):
def ord(x): return x'''
modify function like this,
class RFBDes(pyDes.des):
def setKey(self, key):
"""RFB protocol for authentication requires client to encrypt
challenge sent by server with password using DES method. However,
bits in each byte of the password are put in reverse order before
using it as encryption key."""
newkey = []
for ki in range(len(key)):
tt=key[ki].encode()
bsrc = int.from_bytes(tt,"little")
btgt = 0
for i in range(8):
if bsrc & (1 << i):
btgt = btgt | (1 << 7-i)
newkey.append(chr(btgt))
super(RFBDes, self).setKey(newkey)
That code does not look that pythonic. Depending an #227 if Python 2 support can be dropped this could be rewritten as following:
class RFBDes(pyDes.des):
def setKey(self, key: bytes) -> None:
"""RFB protocol for authentication requires client to encrypt
challenge sent by server with password using DES method. However,
bits in each byte of the password are put in reverse order before
using it as encryption key."""
newkey = bytes(
sum((128 >> i) if (k & (1 << i)) else 0 for i in range(8))
for k in key
)
super(RFBDes, self).setKey(newkey)This requires changing sendPassword() to do the encoding:
def sendPassword(self, password: str) -> None:
"""send password"""
pw = (password + '\0' * 8)[:8] #make sure its 8 chars long, zero padded
des = RFBDes(pw.encode("ASCII"))
response = des.encrypt(self._challenge)
self.transport.write(response)