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

[C#] Fix repeated group buffer overflow #823

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 real-logic:master Nov 26, 2020
@mjpt777
Copy link
Contributor

mjpt777 commented Nov 26, 2020

Thanks

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

Successfully merging this pull request may close these issues.

2 participants