Skip to content

Commit

Permalink
[c#] Fix handling of large container lengths
Browse files Browse the repository at this point in the history 
Fix handling of large container lengths that could cause an infinite
loop when deserializing some payloads. This fix addresses
[CVE-2020-1469](https://msrc-portal-preview.azurewebsites.net/en-US/security-guidance/advisory/CVE-2020-1469).
  • Loading branch information
@chwarr
Amos Ortal authored and chwarr committed Jul 6, 2020
1 parent 963f87a commit b0fd4a1
Show file tree
Hide file tree
Showing 9 changed files with 690 additions and 35 deletions.
3 changes: 3 additions & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,9 @@ different versioning scheme, following the Haskell community's
* Fixed MSB3105/CS2002 error about duplicate Compile items when a directory
contains multiple .bond files and `--gprc` is in `$BondOptions`. ([Issue
#1050](https://github.com/microsoft/bond/issues/1050))
* Fix handling of large container lengths that could cause an infinite loop
when deserializing some payloads. This fix addresses
[CVE-2020-1469](https://msrc-portal-preview.azurewebsites.net/en-US/security-guidance/advisory/CVE-2020-1469).

## 9.0: 2020-05-26 ##
* IDL core version: 3.0
Expand Down
2 changes: 1 addition & 1 deletion cs/src/core/io/safe/InputBuffer.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ public InputBuffer Clone()
/// <exception cref="EndOfStreamException"/>
public void SkipBytes(int count)
{
position += count;
position = checked(position + count);
}

/// <summary>
Expand Down
4 changes: 2 additions & 2 deletions cs/src/core/io/safe/OutputBuffer.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -220,8 +220,8 @@ public virtual void WriteString(Encoding encoding, string value, int size)
// Grow the buffer so that there is enough space to write 'count' bytes
internal virtual void Grow(int count)
{
int minLength = position + count;
length += length >> 1;
int minLength = checked(position + count);
length = checked(length + length >> 1);

const int ArrayIndexMaxValue = 0x7FFFFFC7;
if ((uint)length > ArrayIndexMaxValue)
Expand Down
26 changes: 14 additions & 12 deletions cs/src/core/protocols/CompactBinary.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -210,7 +210,8 @@ public void WriteStructBegin(Metadata metadata)
lengths.RemoveFirst();

output.WriteVarUInt32(length);
PushLengthCheck(output.Position + length);
long position = checked(output.Position + length);
PushLengthCheck(position);
}
}

Expand Down Expand Up @@ -466,8 +467,9 @@ public void WriteWString(string value)
}
else
{
int byteSize = checked(value.Length * 2);
WriteUInt32((UInt32)value.Length);
output.WriteString(Encoding.Unicode, value, value.Length << 1);
output.WriteString(Encoding.Unicode, value, byteSize);
}
}
#endregion
Expand Down Expand Up @@ -629,7 +631,7 @@ public void ReadContainerBegin(out int count, out BondDataType elementType)
if (2 == version && (raw & (0x07 << 5)) != 0)
count = (raw >> 5) - 1;
else
count = (int)input.ReadVarUInt32();
count = checked((int)input.ReadVarUInt32());
}

/// <summary>
Expand All @@ -644,7 +646,7 @@ public void ReadContainerBegin(out int count, out BondDataType keyType, out Bond
{
keyType = (BondDataType)input.ReadUInt8();
valueType = (BondDataType)input.ReadUInt8();
count = (int)input.ReadVarUInt32();
count = checked((int)input.ReadVarUInt32());
}

/// <summary>
Expand Down Expand Up @@ -776,7 +778,7 @@ public double ReadDouble()
[MethodImpl(MethodImplOptions.AggressiveInlining)]
public String ReadString()
{
var length = (int)input.ReadVarUInt32();
var length = checked((int)input.ReadVarUInt32());
return length == 0 ? string.Empty : input.ReadString(Encoding.UTF8, length);
}

Expand All @@ -787,8 +789,8 @@ public String ReadString()
[MethodImpl(MethodImplOptions.AggressiveInlining)]
public string ReadWString()
{
var length = (int)input.ReadVarUInt32();
return length == 0 ? string.Empty : input.ReadString(Encoding.Unicode, length << 1);
var length = checked((int)(input.ReadVarUInt32() * 2));
return length == 0 ? string.Empty : input.ReadString(Encoding.Unicode, length);
}

/// <summary>
Expand Down Expand Up @@ -837,10 +839,10 @@ public void Skip(BondDataType type)
input.ReadVarUInt64();
break;
case (BondDataType.BT_STRING):
input.SkipBytes((int)input.ReadVarUInt32());
input.SkipBytes(checked((int)input.ReadVarUInt32()));
break;
case (BondDataType.BT_WSTRING):
input.SkipBytes((int)(input.ReadVarUInt32() << 1));
input.SkipBytes(checked((int)(input.ReadVarUInt32() *2)));
break;
case BondDataType.BT_LIST:
case BondDataType.BT_SET:
Expand Down Expand Up @@ -872,11 +874,11 @@ void SkipContainer()
}
else if (elementType == BondDataType.BT_FLOAT)
{
input.SkipBytes(count * sizeof(float));
input.SkipBytes(checked(count * sizeof(float)));
}
else if (elementType == BondDataType.BT_DOUBLE)
{
input.SkipBytes(count * sizeof(double));
input.SkipBytes(checked(count * sizeof(double)));
}
else
{
Expand Down Expand Up @@ -905,7 +907,7 @@ void SkipStruct()
{
if (2 == version)
{
input.SkipBytes((int)input.ReadVarUInt32());
input.SkipBytes(checked((int)input.ReadVarUInt32()));
}
else
{
Expand Down
6 changes: 4 additions & 2 deletions cs/src/core/protocols/CompactBinaryCounter.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,9 @@ private CounterStackFrame GetCurrentStackFrame()

private void AddBytes(int count)
{
GetCurrentStackFrame().currentLength += count;
var stackFrame = GetCurrentStackFrame();
var length = checked(stackFrame.currentLength + count);
stackFrame.currentLength = length;
}

private void AddVarUInt16(ushort value)
Expand Down Expand Up @@ -99,7 +101,7 @@ public void WriteStructEnd()
if (counterStack.Count > 0)
{
AddVarUInt32(structLength);
AddBytes((int)structLength);
AddBytes(checked((int)structLength));
}
}

Expand Down
27 changes: 14 additions & 13 deletions cs/src/core/protocols/FastBinary.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -367,8 +367,9 @@ public void WriteWString(string value)
}
else
{
int byteSize = checked(value.Length * 2);
output.WriteVarUInt32((UInt32)value.Length);
output.WriteString(Encoding.Unicode, value, value.Length << 1);
output.WriteString(Encoding.Unicode, value, byteSize);
}
}
#endregion
Expand Down Expand Up @@ -478,7 +479,7 @@ public void ReadFieldEnd()
public void ReadContainerBegin(out int count, out BondDataType elementType)
{
elementType = (BondDataType)input.ReadUInt8();
count = (int)input.ReadVarUInt32();
count = checked((int)input.ReadVarUInt32());
}

/// <summary>
Expand All @@ -493,7 +494,7 @@ public void ReadContainerBegin(out int count, out BondDataType keyType, out Bond
{
keyType = (BondDataType)input.ReadUInt8();
valueType = (BondDataType)input.ReadUInt8();
count = (int)input.ReadVarUInt32();
count = checked((int)input.ReadVarUInt32());
}

/// <summary>
Expand Down Expand Up @@ -625,7 +626,7 @@ public double ReadDouble()
[MethodImpl(MethodImplOptions.AggressiveInlining)]
public String ReadString()
{
var length = (int)input.ReadVarUInt32();
var length = checked((int)input.ReadVarUInt32());
return length == 0 ? string.Empty : input.ReadString(Encoding.UTF8, length);
}

Expand All @@ -636,8 +637,8 @@ public String ReadString()
[MethodImpl(MethodImplOptions.AggressiveInlining)]
public string ReadWString()
{
var length = (int)input.ReadVarUInt32();
return length == 0 ? string.Empty : input.ReadString(Encoding.Unicode, length << 1);
var length = checked((int)(input.ReadVarUInt32() * 2));
return length == 0 ? string.Empty : input.ReadString(Encoding.Unicode, length);
}

/// <summary>
Expand Down Expand Up @@ -686,10 +687,10 @@ public void Skip(BondDataType type)
input.SkipBytes(sizeof(ulong));
break;
case (BondDataType.BT_STRING):
input.SkipBytes((int)input.ReadVarUInt32());
input.SkipBytes(checked(((int)input.ReadVarUInt32())));
break;
case (BondDataType.BT_WSTRING):
input.SkipBytes((int)(input.ReadVarUInt32() << 1));
input.SkipBytes(checked((int)(input.ReadVarUInt32()) * 2));
break;
case BondDataType.BT_LIST:
case BondDataType.BT_SET:
Expand Down Expand Up @@ -724,21 +725,21 @@ void SkipContainer()
break;
case (BondDataType.BT_UINT16):
case (BondDataType.BT_INT16):
input.SkipBytes(count * sizeof(ushort));
input.SkipBytes(checked(count * sizeof(ushort)));
break;
case (BondDataType.BT_UINT32):
case (BondDataType.BT_INT32):
input.SkipBytes(count * sizeof(uint));
input.SkipBytes(checked(count * sizeof(uint)));
break;
case (BondDataType.BT_UINT64):
case (BondDataType.BT_INT64):
input.SkipBytes(count * sizeof(ulong));
input.SkipBytes(checked(count * sizeof(ulong)));
break;
case BondDataType.BT_FLOAT:
input.SkipBytes(count * sizeof(float));
input.SkipBytes(checked(count * sizeof(float)));
break;
case BondDataType.BT_DOUBLE:
input.SkipBytes(count * sizeof(double));
input.SkipBytes(checked(count * sizeof(double)));
break;
default:
while (0 <= --count)
Expand Down
8 changes: 4 additions & 4 deletions cs/src/core/protocols/SimpleBinary.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -356,7 +356,7 @@ public void WriteWString(string value)
else
{
WriteLength(value.Length);
output.WriteString(Encoding.Unicode, value, value.Length << 1);
output.WriteString(Encoding.Unicode, value, checked(value.Length * 2));
}
}
#endregion
Expand Down Expand Up @@ -628,13 +628,13 @@ public void SkipString()
public string ReadWString()
{
var length = ReadLength();
return length == 0 ? string.Empty : input.ReadString(Encoding.Unicode, length << 1);
return length == 0 ? string.Empty : input.ReadString(Encoding.Unicode, checked(length * 2));
}

[MethodImpl(MethodImplOptions.AggressiveInlining)]
public void SkipWString()
{
input.SkipBytes(ReadLength() << 1);
input.SkipBytes(checked(ReadLength() * 2));
}

/// <summary>
Expand All @@ -659,7 +659,7 @@ public void SkipBytes(int count)
[MethodImpl(MethodImplOptions.AggressiveInlining)]
int ReadLength()
{
return (int)((version == 2) ? input.ReadVarUInt32() : input.ReadUInt32());
return (version == 2) ? checked((int)input.ReadVarUInt32()) : checked((int)input.ReadUInt32());
}
}
}
2 changes: 1 addition & 1 deletion cs/src/io/unsafe/InputPointer.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ public long Position
/// <param name="count">Number of bytes to skip</param>
public void SkipBytes(int count)
{
position += count;
position = checked(position + count);
}

/// <summary>
Expand Down
Loading

0 comments on commit b0fd4a1

Please sign in to comment.