Skip to content

Commit

Permalink
Fix WriteNamedBitList for enums with negative values.
Browse files Browse the repository at this point in the history 
WriteNamedBitList internally operates on a ulong when writing down bits. If the enum is backed by a signed type and the enum value is negative, it would be signed extended when being converted to a long, then ulong. This resulted in all negative values being written down as 64 bits set regardless of the width of the enum type.

This changes the write to instead convert to the signed types corresponding unsigned type first, then converting to a ulong, which results in the value being zero extended.
  • Loading branch information
@vcsjones
vcsjones authored Jan 4, 2025
1 parent fb7fe3e commit 0f6c3d8
Show file tree
Hide file tree
Showing 3 changed files with 139 additions and 30 deletions.
23 changes: 19 additions & 4 deletions src/libraries/System.Formats.Asn1/src/System/Formats/Asn1/AsnWriter.NamedBitList.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -119,15 +119,30 @@ private void WriteNamedBitList(Asn1Tag? tag, Type tEnum, Enum value)

ulong integralValue;

if (backingType == typeof(ulong))
// When widening from a signed type to a ulong it must zero extend not sign extend. Convert to unsigned
// types first for zero extension.
if (backingType == typeof(sbyte))
{
integralValue = unchecked((byte)Convert.ToSByte(value));
}
else if (backingType == typeof(short))
{
integralValue = unchecked((ushort)Convert.ToInt16(value));
}
else if (backingType == typeof(int))
{
integralValue = unchecked((uint)Convert.ToInt32(value));
}
else if (backingType == typeof(ulong))
{
// long is handled in the catch all, this handles ulong specifically since it may not fit in a long.
integralValue = Convert.ToUInt64(value);
}
else
{
// All other types fit in a (signed) long.
long numericValue = Convert.ToInt64(value);
integralValue = unchecked((ulong)numericValue);
// Other unsigned types fit in a long without concern for their sign.
// This also handles a signed long, which doesn't need to be widened.
integralValue = unchecked((ulong)Convert.ToInt64(value));
}

WriteNamedBitList(tag, integralValue);
Expand Down
67 changes: 67 additions & 0 deletions src/libraries/System.Formats.Asn1/tests/Reader/ReadNamedBitList.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,61 @@ public enum LongFlags : long
Mid = 1L << 32,
Max = 1L << 62,
Min = long.MinValue,
AllBits = -1
}

[Flags]
public enum IntFlags : int
{
None = 0,
Mid = 1 << 16,
Max = int.MaxValue,
AllBits = -1,
}

[Flags]
public enum UIntFlags : uint
{
None = 0,
Mid = 1 << 16,
Max = uint.MaxValue,
Min = uint.MinValue,
}

[Flags]
public enum ShortFlags : short
{
None = 0,
Mid = 1 << 8,
Max = short.MaxValue,
AllBits = -1,
}

[Flags]
public enum UShortFlags : ushort
{
None = 0,
Mid = 1 << 8,
Max = ushort.MaxValue,
Min = ushort.MinValue,
}

[Flags]
public enum ByteFlags : byte
{
None = 0,
Mid = 1 << 4,
Max = byte.MaxValue,
Min = byte.MinValue,
}

[Flags]
public enum SByteFlags : sbyte
{
None = 0,
Mid = 1 << 4,
Max = sbyte.MaxValue,
AllBits = -1,
}

[Theory]
Expand Down Expand Up @@ -94,6 +149,18 @@ public enum LongFlags : long
typeof(X509KeyUsageCSharpStyle),
(long)(X509KeyUsageCSharpStyle.DecipherOnly | X509KeyUsageCSharpStyle.KeyCertSign | X509KeyUsageCSharpStyle.DataEncipherment),
"0303001480")]
[InlineData(AsnEncodingRules.DER, typeof(SByteFlags), (sbyte)SByteFlags.AllBits, "030200FF")]
[InlineData(AsnEncodingRules.CER, typeof(SByteFlags), (sbyte)SByteFlags.AllBits, "030200FF")]
[InlineData(AsnEncodingRules.BER, typeof(SByteFlags), (sbyte)SByteFlags.AllBits, "030200FF")]
[InlineData(AsnEncodingRules.DER, typeof(ShortFlags), (short)ShortFlags.AllBits, "030300FFFF")]
[InlineData(AsnEncodingRules.CER, typeof(ShortFlags), (short)ShortFlags.AllBits, "030300FFFF")]
[InlineData(AsnEncodingRules.BER, typeof(ShortFlags), (short)ShortFlags.AllBits, "030300FFFF")]
[InlineData(AsnEncodingRules.DER, typeof(IntFlags), (int)IntFlags.AllBits, "030500FFFFFFFF")]
[InlineData(AsnEncodingRules.CER, typeof(IntFlags), (int)IntFlags.AllBits, "030500FFFFFFFF")]
[InlineData(AsnEncodingRules.BER, typeof(IntFlags), (int)IntFlags.AllBits, "030500FFFFFFFF")]
[InlineData(AsnEncodingRules.DER, typeof(LongFlags), (long)LongFlags.AllBits, "030900FFFFFFFFFFFFFFFF")]
[InlineData(AsnEncodingRules.CER, typeof(LongFlags), (long)LongFlags.AllBits, "030900FFFFFFFFFFFFFFFF")]
[InlineData(AsnEncodingRules.BER, typeof(LongFlags), (long)LongFlags.AllBits, "030900FFFFFFFFFFFFFFFF")]
public static void VerifyReadNamedBitListEncodings(
AsnEncodingRules ruleSet,
Type enumType,
Expand Down
79 changes: 53 additions & 26 deletions src/libraries/System.Formats.Asn1/tests/Writer/WriteNamedBitList.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
// The .NET Foundation licenses this file to you under the MIT license.

using System.Collections;
using System.Collections.Generic;
using System.Formats.Asn1.Tests.Reader;
using System.Security.Cryptography.X509Certificates;
using Test.Cryptography;
Expand All @@ -12,32 +13,7 @@ namespace System.Formats.Asn1.Tests.Writer
public class WriteNamedBitList : Asn1WriterTests
{
[Theory]
[InlineData(
AsnEncodingRules.BER,
"030100",
ReadNamedBitList.ULongFlags.None)]
[InlineData(
AsnEncodingRules.CER,
"030100",
ReadNamedBitList.ULongFlags.None)]
[InlineData(
AsnEncodingRules.DER,
"030100",
ReadNamedBitList.ULongFlags.None)]
[InlineData(
AsnEncodingRules.BER,
"0309000000000000000003",
ReadNamedBitList.ULongFlags.Max | ReadNamedBitList.ULongFlags.AlmostMax)]
[InlineData(
AsnEncodingRules.CER,
"0309010000000080000002",
ReadNamedBitList.LongFlags.Max | ReadNamedBitList.LongFlags.Mid)]
[InlineData(
AsnEncodingRules.DER,
"030204B0",
ReadNamedBitList.X509KeyUsageCSharpStyle.DigitalSignature |
ReadNamedBitList.X509KeyUsageCSharpStyle.KeyEncipherment |
ReadNamedBitList.X509KeyUsageCSharpStyle.DataEncipherment)]
[MemberData(nameof(VerifyWriteNamedBitListTheories))]
public static void VerifyWriteNamedBitList(
AsnEncodingRules ruleSet,
string expectedHex,
Expand Down Expand Up @@ -380,5 +356,56 @@ public static void VerifyWriteNamedBitList_KeyUsage_TwoByte(AsnEncodingRules rul

Verify(writer, kuExt.RawData.ByteArrayToHex());
}

public static IEnumerable<object[]> VerifyWriteNamedBitListTheories
{
get
{
AsnEncodingRules[] rules = [AsnEncodingRules.BER, AsnEncodingRules.CER, AsnEncodingRules.DER];

foreach (AsnEncodingRules rule in rules)
{
yield return new object[] { rule, "030100", ReadNamedBitList.ULongFlags.None };
yield return new object[] { rule, "030100", ReadNamedBitList.LongFlags.None };

yield return new object[] { rule, "030100", ReadNamedBitList.UIntFlags.None };
yield return new object[] { rule, "030100", ReadNamedBitList.IntFlags.None };

yield return new object[] { rule, "030100", ReadNamedBitList.UShortFlags.None };
yield return new object[] { rule, "030100", ReadNamedBitList.ShortFlags.None };

yield return new object[] { rule, "030100", ReadNamedBitList.ByteFlags.None };
yield return new object[] { rule, "030100", ReadNamedBitList.SByteFlags.None };

yield return new object[] { rule, "030200FF", ReadNamedBitList.SByteFlags.AllBits };
yield return new object[] { rule, "030300FFFF", ReadNamedBitList.ShortFlags.AllBits };
yield return new object[] { rule, "030500FFFFFFFF", ReadNamedBitList.IntFlags.AllBits };
yield return new object[] { rule, "030900FFFFFFFFFFFFFFFF", ReadNamedBitList.LongFlags.AllBits };
}

yield return new object[]
{
AsnEncodingRules.BER,
"0309000000000000000003",
ReadNamedBitList.ULongFlags.Max | ReadNamedBitList.ULongFlags.AlmostMax,
};

yield return new object[]
{
AsnEncodingRules.CER,
"0309010000000080000002",
ReadNamedBitList.LongFlags.Max | ReadNamedBitList.LongFlags.Mid,
};

yield return new object[]
{
AsnEncodingRules.DER,
"030204B0",
ReadNamedBitList.X509KeyUsageCSharpStyle.DigitalSignature |
ReadNamedBitList.X509KeyUsageCSharpStyle.KeyEncipherment |
ReadNamedBitList.X509KeyUsageCSharpStyle.DataEncipherment,
};
}
}
}
}

0 comments on commit 0f6c3d8

Please sign in to comment.