Skip to content

[C#] Fix repeated group buffer overflow #823

New issue

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

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

Already on GitHub? # to your account

Merged
merged 1 commit into from
Nov 26, 2020
Merged

[C#] Fix repeated group buffer overflow #823

merged 1 commit into from
Nov 26, 2020

Conversation

j4b6ski
Copy link
Contributor

@j4b6ski j4b6ski commented Nov 26, 2020

This actually broke code I was working on in a very nasty way when the buffer size was too small - I was trying to use the DirectBuffer's bufferOverflow delegate and some bytes were getting eaten when expanding the buffer.

Explanation in the commit msg:

On a repeated group in the WrapForEncode, the parentMessage.Limit has two purposes:

  1. It is used as a buffer offset for the _dimensions helper Wrap method
  2. The assignment to it has a side effect of checking that we do not exceed the underlying DirectBuffer's capacity

However notice that currently this assignment happens after writing to the _dimensions helper properties BlockLength and NumInGroup,
i.e. the writes are done before we check for buffer overflow.
Underneath these are direct writes to the byte* managed by the DirectBuffer, resulting in an unchecked, unsafe buffer overflow.
To fix this the Limit has to be increased right after the call to the _dimensions.Wrap, before writes to BlockLength and NumInGroup

The same occurs in the Decode method, only reading unsafe, invalid memory instead of writing to it.
Also fixed _actingVersion was being used before it's assigned in the Encode

On a repeated group in the WrapForEncode, the parentMessage.Limit has two purposes:
1. It is used as a buffer offset for the _dimensions helper Wrap method
2. The assignment to it has a side effect of checking that we do not exceed the underlying DirectBuffer's capacity
However notice that currently this assignment happens after writing to the _dimensions helper properties BlockLength and NumInGroup,
i.e. the writes are done *before* we check for buffer overflow. 
Underneath these are direct writes to the byte* managed by the DirectBuffer, resulting in an unchecked, unsafe buffer overflow.
To fix this the Limit has to be increased right after the call to the _dimensions.Wrap, before writes to BlockLength and NumInGroup

The same occurs in the Decode method, only reading unsafe, invalid memory instead of writing to it.
Also fixed _actingVersion was being used before it's assigned in the Encode
@mjpt777 mjpt777 merged commit 4437ae5 into aeron-io:master Nov 26, 2020
@mjpt777
Copy link
Contributor

mjpt777 commented Nov 26, 2020

Thanks

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants