Skip to content

Support full uint range for body length #638

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.

Sign up for GitHub

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Support full uint range for body length #638

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 15 additions & 13 deletions crypto/src/bcpg/BcpgInputStream.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -120,8 +120,7 @@ public Packet ReadPacket()

bool newPacket = (hdr & 0x40) != 0;
PacketTag tag = 0;
// TODO[pgp] Is the length field supposed to support full uint range?
int bodyLen;
uint bodyLen;
bool partial = false;

if (newPacket)
Expand All @@ -145,7 +144,7 @@ public Packet ReadPacket()
bodyLen = StreamUtilities.RequireUInt16BE(this);
break;
case 2:
bodyLen = (int)StreamUtilities.RequireUInt32BE(this);
bodyLen = StreamUtilities.RequireUInt32BE(this);
break;
case 3:
bodyLen = 0;
Expand Down Expand Up @@ -238,19 +237,20 @@ protected override void Dispose(bool disposing)

/// <summary>
/// A stream that overlays our input stream, allowing the user to only read a segment of it.
/// NB: dataLength will be negative if the segment length is in the upper range above 2**31.
/// If partial is true, dataLength should only be up to 2^30 bytes.
/// Otherwise it's a non-partial packet and can be larger.
/// </summary>
private class PartialInputStream
: BaseInputStream
{
private BcpgInputStream m_in;
private bool partial;
private int dataLength;
private uint dataLength;

internal PartialInputStream(
BcpgInputStream bcpgIn,
bool partial,
int dataLength)
uint dataLength)
{
this.m_in = bcpgIn;
this.partial = partial;
Expand Down Expand Up @@ -284,12 +284,12 @@ public override int Read(byte[] buffer, int offset, int count)
{
if (dataLength != 0)
{
int readLen = (dataLength > count || dataLength < 0) ? count : dataLength;
int readLen = (dataLength > count) ? count : (int)dataLength;
int len = m_in.Read(buffer, offset, readLen);
if (len < 1)
throw new EndOfStreamException("Premature end of stream in PartialInputStream");

dataLength -= len;
dataLength = (uint)(dataLength - len);
return len;
}
}
Expand All @@ -306,12 +306,12 @@ public override int Read(Span<byte> buffer)
if (dataLength != 0)
{
int count = buffer.Length;
int readLen = (dataLength > count || dataLength < 0) ? count : dataLength;
int readLen = (dataLength > count) ? count : (int)dataLength;
int len = m_in.Read(buffer[..readLen]);
if (len < 1)
throw new EndOfStreamException("Premature end of stream in PartialInputStream");

dataLength -= len;
dataLength = (uint)(dataLength - len);
return len;
}
}
Expand All @@ -323,16 +323,18 @@ public override int Read(Span<byte> buffer)

private bool ReadPartialDataLength()
{
int bodyLen = StreamUtilities.ReadBodyLen(m_in, out var streamFlags);
if (bodyLen < 0)
uint? bodyLen = StreamUtilities.ReadBodyLen(m_in, out var streamFlags);
// If we can't read the body length, or it's too large, we can't continue
// reading the stream as a partial stream, because partials only support up to 2^30
if (!bodyLen.HasValue || bodyLen > (1U << 30))
{
partial = false;
dataLength = 0;
return false;
}

partial = streamFlags.HasFlag(StreamUtilities.StreamFlags.Partial);
dataLength = bodyLen;
dataLength = bodyLen.Value;
return true;
}
}
Expand Down
8 changes: 4 additions & 4 deletions crypto/src/bcpg/SignatureSubpacketsReader.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,21 +20,21 @@ public SignatureSubpacketsParser(Stream input)

public SignatureSubpacket ReadPacket()
{
int bodyLen = StreamUtilities.ReadBodyLen(m_input, out var streamFlags);
if (bodyLen < 0)
uint? bodyLen = StreamUtilities.ReadBodyLen(m_input, out var streamFlags);
if (!bodyLen.HasValue)
return null;

if (streamFlags.HasFlag(StreamUtilities.StreamFlags.Partial))
throw new IOException("unexpected length header");

bool isLongLength = streamFlags.HasFlag(StreamUtilities.StreamFlags.LongLength);

if (bodyLen < 1)
if (bodyLen.Value < 1)
throw new EndOfStreamException("out of range data found in signature sub packet");

int tag = StreamUtilities.RequireByte(m_input);

byte[] data = new byte[bodyLen - 1];
byte[] data = new byte[bodyLen.Value - 1];

//
// this may seem a bit strange but it turns out some applications miscode the length
Expand Down
28 changes: 17 additions & 11 deletions crypto/src/bcpg/StreamUtilities.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,39 +16,45 @@ internal enum StreamFlags
Partial = 2,
}

internal static int ReadBodyLen(Stream s, out StreamFlags flags)
internal static uint? ReadBodyLen(Stream s, out StreamFlags flags)
{
flags = StreamFlags.None;

int b0 = s.ReadByte();
if (b0 < 0)
return -1;
return null;

if (b0 < 192)
return b0;
return (uint)b0;

if (b0 < 224)
{
int b1 = RequireByte(s);
return ((b0 - 192) << 8) + b1 + 192;
return (uint)(((b0 - 192) << 8) + b1 + 192);
}

if (b0 == 255)
{
flags |= StreamFlags.LongLength;
return (int)RequireUInt32BE(s);
return RequireUInt32BE(s);
}

flags |= StreamFlags.Partial;
return 1 << (b0 & 0x1F);
if (b0 >= 224 && b0 < 255)
{
flags |= StreamFlags.Partial;
return (uint)1 << (b0 & 0x1F);
}

// Invalid value
return null;
}

internal static int RequireBodyLen(Stream s, out StreamFlags flags)
internal static uint RequireBodyLen(Stream s, out StreamFlags flags)
{
int bodyLen = ReadBodyLen(s, out flags);
if (bodyLen < 0)
uint? bodyLen = ReadBodyLen(s, out flags);
if (!bodyLen.HasValue)
throw new EndOfStreamException();
return bodyLen;
return bodyLen.Value;
}

internal static byte RequireByte(Stream s)
Expand Down
6 changes: 3 additions & 3 deletions crypto/src/bcpg/UserAttributeSubpacketsReader.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,8 @@ public UserAttributeSubpacketsParser(Stream input)

public virtual UserAttributeSubpacket ReadPacket()
{
int bodyLen = StreamUtilities.ReadBodyLen(m_input, out var streamFlags);
if (bodyLen < 0)
uint? bodyLen = StreamUtilities.ReadBodyLen(m_input, out var streamFlags);
if (!bodyLen.HasValue)
return null;

if (streamFlags.HasFlag(StreamUtilities.StreamFlags.Partial))
Expand All @@ -29,7 +29,7 @@ public virtual UserAttributeSubpacket ReadPacket()
bool isLongLength = streamFlags.HasFlag(StreamUtilities.StreamFlags.LongLength);

int tag = StreamUtilities.RequireByte(m_input);
byte[] data = new byte[bodyLen - 1];
byte[] data = new byte[bodyLen.Value - 1];
StreamUtilities.RequireBytes(m_input, data);

UserAttributeSubpacketTag type = (UserAttributeSubpacketTag)tag;
Expand Down