From 418ed80c623e676eb2680011f5b3f02b22a9b89e Mon Sep 17 00:00:00 2001 From: Ian Hays Date: Mon, 24 Oct 2016 13:09:36 -0700 Subject: [PATCH] Add some extra checks to BinaryReader/Writer buffers --- src/mscorlib/src/System/IO/BinaryReader.cs | 24 ++++++++++++++++++++---- src/mscorlib/src/System/IO/BinaryWriter.cs | 25 ++++++++++++++++++------- 2 files changed, 38 insertions(+), 11 deletions(-) diff --git a/src/mscorlib/src/System/IO/BinaryReader.cs b/src/mscorlib/src/System/IO/BinaryReader.cs index c5136f3..32e26c0 100644 --- a/src/mscorlib/src/System/IO/BinaryReader.cs +++ b/src/mscorlib/src/System/IO/BinaryReader.cs @@ -372,10 +372,26 @@ namespace System.IO { } Contract.Assert(byteBuffer != null, "expected byteBuffer to be non-null"); - unsafe { - fixed (byte* pBytes = byteBuffer) - fixed (char* pChars = buffer) { - charsRead = m_decoder.GetChars(pBytes + position, numBytes, pChars + index, charsRemaining, false); + + checked + { + if (position < 0 || numBytes < 0 || position + numBytes > byteBuffer.Length) + { + throw new ArgumentOutOfRangeException(nameof(byteCount)); + } + if (index < 0 || charsRemaining < 0 || index + charsRemaining > buffer.Length) + { + throw new ArgumentOutOfRangeException(nameof(charsRemaining)); + } + unsafe + { + fixed (byte* pBytes = byteBuffer) + { + fixed (char* pChars = buffer) + { + charsRead = m_decoder.GetChars(pBytes + position, numBytes, pChars + index, charsRemaining, false); + } + } } } diff --git a/src/mscorlib/src/System/IO/BinaryWriter.cs b/src/mscorlib/src/System/IO/BinaryWriter.cs index 6a80bb2..03fe51f 100644 --- a/src/mscorlib/src/System/IO/BinaryWriter.cs +++ b/src/mscorlib/src/System/IO/BinaryWriter.cs @@ -194,7 +194,7 @@ namespace System.IO { Contract.Assert(_encoding.GetMaxByteCount(1) <= 16, "_encoding.GetMaxByteCount(1) <= 16)"); int numBytes = 0; fixed(byte * pBytes = _buffer) { - numBytes = _encoder.GetBytes(&ch, 1, pBytes, 16, true); + numBytes = _encoder.GetBytes(&ch, 1, pBytes, _buffer.Length, true); } OutStream.Write(_buffer, 0, numBytes); } @@ -361,10 +361,11 @@ namespace System.IO { if (_largeByteBuffer == null) { _largeByteBuffer = new byte[LargeByteBufferSize]; - _maxChars = LargeByteBufferSize / _encoding.GetMaxByteCount(1); + _maxChars = _largeByteBuffer.Length / _encoding.GetMaxByteCount(1); } - if (len <= LargeByteBufferSize) { + if (len <= _largeByteBuffer.Length) + { //Contract.Assert(len == _encoding.GetBytes(chars, 0, chars.Length, _largeByteBuffer, 0), "encoding's GetByteCount & GetBytes gave different answers! encoding type: "+_encoding.GetType().Name); _encoding.GetBytes(value, 0, value.Length, _largeByteBuffer, 0); OutStream.Write(_largeByteBuffer, 0, len); @@ -383,14 +384,24 @@ namespace System.IO { // Figure out how many chars to process this round. int charCount = (numLeft > _maxChars) ? _maxChars : numLeft; int byteLen; - fixed(char* pChars = value) { - fixed(byte* pBytes = _largeByteBuffer) { - byteLen = _encoder.GetBytes(pChars + charStart, charCount, pBytes, LargeByteBufferSize, charCount == numLeft); + + checked + { + if (charStart < 0 || charCount < 0 || charStart + charCount > value.Length) + { + throw new ArgumentOutOfRangeException(nameof(charCount)); + } + fixed (char* pChars = value) + { + fixed (byte* pBytes = _largeByteBuffer) + { + byteLen = _encoder.GetBytes(pChars + charStart, charCount, pBytes, _largeByteBuffer.Length, charCount == numLeft); + } } } #if _DEBUG totalBytes += byteLen; - Contract.Assert (totalBytes <= len && byteLen <= LargeByteBufferSize, "BinaryWriter::Write(String) - More bytes encoded than expected!"); + Contract.Assert (totalBytes <= len && byteLen <= _largeByteBuffer.Length, "BinaryWriter::Write(String) - More bytes encoded than expected!"); #endif OutStream.Write(_largeByteBuffer, 0, byteLen); charStart += charCount; -- 2.7.4