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

fix(Net) bad mask with odd number of bytes #4838

Merged

Conversation

d3matt
Copy link

@d3matt d3matt commented Jan 6, 2025

Receiving an odd number of bytes in non-blocking mode results in using the wrong bytes to unmask the payload. Keep track of the number of payload bytes that have been unmasked.

Receiving an odd number of bytes in non-blocking mode results in using the wrong bytes to unmask the payload. Keep track of the number of payload bytes that have been unmasked.
@d3matt
Copy link
Author

d3matt commented Jan 7, 2025

I'm definitely open to suggestions on how to unit test this... Ideally, I'd have a test that sends in the payload of a frame one byte at a time (setting TCP_NODELAY so each byte wakes up the receiving end).

@obiltschnig obiltschnig self-assigned this Jan 7, 2025
@obiltschnig obiltschnig added the bug label Jan 7, 2025
@obiltschnig obiltschnig added this to the Release 1.14.1 milestone Jan 7, 2025
@obiltschnig
Copy link
Member

I would take _receiveState.maskOffset modulo MASK_LENGTH to prevent an overflow issue with frame sizes that don't fit into int.

@@ -297,7 +297,7 @@ int WebSocketImpl::receiveBytes(void* buffer, int length, int)

skipHeader(_receiveState.headerLength);

if (receivePayload(reinterpret_cast<char*>(buffer), payloadLength, _receiveState.mask, _receiveState.useMask) != payloadLength)
if (receivePayload(reinterpret_cast<char*>(buffer), payloadLength, _receiveState.mask, _receiveState.useMask, 0) != payloadLength)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For consistency I would pass in _receiveState.maskOffset here as well instead of 0.

@obiltschnig obiltschnig merged commit 5652837 into pocoproject:main Jan 8, 2025
34 of 35 checks passed
matejk pushed a commit that referenced this pull request Jan 14, 2025
…t_mask

fix(Net) bad mask with odd number of bytes
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants