Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SECURITY: heap buffer overflow inside structure in libvncclient #250

Closed
PaulCher opened this issue Sep 11, 2018 · 3 comments

Comments

@PaulCher
Copy link

commented Sep 11, 2018

Buffer inside rfbClient structure has fixed size in rfbclient.h

char buffer[RFB_BUFFER_SIZE];
But HandleCorreXXX function in corre.c
if (!ReadFromRFBServer(client, client->buffer, hdr.nSubrects * (4 + (BPP / 8))))
does not check size of the buffer and reads into it amount of bytes equal to value receivied from network times (4 + (XXX/8)). XXX is constant value in [8, 16, 24, 32]

I am sure that this issue is exploitable, because I managed to achieve remote code execution in the product which uses libvncclient as a third party for a Proof of Concept.

This security issue is a result of my work at Kaspersky Lab ICS CERT Vulnerability Research Group at position of Security Researcher.

For more information about ICS CERT please contact:
ics-cert@kaspersky.com
https://ics-cert.kaspersky.com/

Best regards,
Pavel Cheremushkin

@bk138 bk138 self-assigned this Sep 29, 2018

@bk138 bk138 closed this in 09f2f3f Sep 29, 2018

@PaulCher

This comment has been minimized.

Copy link
Author

commented Oct 1, 2018

The fix is not correct.

if (hdr.nSubrects * (4 + (BPP / 8)) > RFB_BUFFER_SIZE || !ReadFromRFBServer(client, client->buffer, hdr.nSubrects * (4 + (BPP / 8))))

Suggest value hdr.nSubrects equals 0x20000001 , BPP is 32, then hdr.nSubrects * 8 would overflow 32 bit int, and will be equal 8, so that this value will bypass the check. Only 8 bytes will be read into the buffer, but after the cycle for (i = 0; i < hdr.nSubrects; i++) will overflow again.

That's why I suggested the following fix:
https://github.com/PaulCher/libvncserver/blob/4a0dc9cc9ecd0cf0c654c6fc94ac3ca77b2eea26/libvncclient/corre.c#L45-L46

@bk138 bk138 reopened this Oct 1, 2018

@bk138 bk138 added this to the Release 0.9.12 milestone Oct 1, 2018

@bk138

This comment has been minimized.

Copy link
Member

commented Oct 1, 2018

@PaulCher

This comment has been minimized.

Copy link
Author

commented Oct 4, 2018

Yeah, my bad. >= will be an overkill in this case, seems > should be okay.

@bk138 bk138 closed this in 7b1ef0f Oct 4, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.