Skip to content

Commit

Permalink
[c#] Fix OutputBuffer.Grow geometric growth regression
Browse files Browse the repository at this point in the history
Recently, in order to mitigate CVE-2020-1469, among other changes in
b0fd4a1 ([c#] Fix handling of large container lengths, 2020-07-06),
`OutputBuffer.Grow` changed from `length += length >> 1` to `length =
checked(length + length >> 1)`. This actually doesn't change the value
of `length`, because `+` has [higher precedence][1] than `>>`. `length`
gets doubled and then divided by two, so it remains unchanged (except
when overflow occurs). This causes serious performance regression as the
underlying buffer is then grown by single byte at a time instead of
growing by half of current size.

The fix is to use `length = checked(length + (length >> 1))` to
explicitly perform the shift before the addition.

Fixes #1065
Closes #1066

[1]: https://docs.microsoft.com/en-us/dotnet/csharp/language-reference/operators/#operator-precedence
  • Loading branch information
Eitan Har-Shoshanim authored and chwarr committed Aug 6, 2020
1 parent 473e589 commit 3afea82
Show file tree
Hide file tree
Showing 2 changed files with 15 additions and 1 deletion.
14 changes: 14 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,20 @@ tag versions. The Bond compiler (`gbc`) and
different versioning scheme, following the Haskell community's
[package versioning policy](https://wiki.haskell.org/Package_versioning_policy).

## Unreleased ##

* IDL core version: TBD
* C++ version: TBD
* C# NuGet version: bug fix bump needed
* `gbc` & compiler library: TBD

### C# ###

* Fixed a performance regression in `OutputBuffer.Grow`: it was incorrectly
growing the buffer by one byte at a time instead of geometrically. ([Issue
\#1065](https://github.com/microsoft/bond/issues/1065), [Pull request
\#1066](https://github.com/microsoft/bond/pull/1066))

## 9.0.2: 2020-08-03 ##
* IDL core version: 3.0
* C++ version: 9.0.2
Expand Down
2 changes: 1 addition & 1 deletion cs/src/core/io/safe/OutputBuffer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -221,7 +221,7 @@ public virtual void WriteString(Encoding encoding, string value, int size)
internal virtual void Grow(int count)
{
int minLength = checked(position + count);
length = checked(length + length >> 1);
length = checked(length + (length >> 1));

const int ArrayIndexMaxValue = 0x7FFFFFC7;
if ((uint)length > ArrayIndexMaxValue)
Expand Down

0 comments on commit 3afea82

Please sign in to comment.