From 3afea822c42dd0095fedb9e7db9ebb99165e7343 Mon Sep 17 00:00:00 2001 From: Eitan Har-Shoshanim Date: Thu, 6 Aug 2020 18:04:14 +0300 Subject: [PATCH] [c#] Fix OutputBuffer.Grow geometric growth regression Recently, in order to mitigate CVE-2020-1469, among other changes in b0fd4a15a ([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 https://github.com/microsoft/bond/issues/1065 Closes https://github.com/microsoft/bond/pull/1066 [1]: https://docs.microsoft.com/en-us/dotnet/csharp/language-reference/operators/#operator-precedence --- CHANGELOG.md | 14 ++++++++++++++ cs/src/core/io/safe/OutputBuffer.cs | 2 +- 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e8eb99ff87..13f6311b32 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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 diff --git a/cs/src/core/io/safe/OutputBuffer.cs b/cs/src/core/io/safe/OutputBuffer.cs index 7ad470042f..1fd83138c7 100644 --- a/cs/src/core/io/safe/OutputBuffer.cs +++ b/cs/src/core/io/safe/OutputBuffer.cs @@ -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)