Skip to content

Commit 6abb139

Browse files
bartonjsbartonjs
committed
Fix the type of exception thrown when decoding bad headers
CoseMessage's Decode routines say that any failure is a CryptographicException, but some of the validation failures leak out ArgumentException from the validation in CoseHeaderMap.
1 parent 2805c1d commit 6abb139

File tree

4 files changed

+333
-29
lines changed

4 files changed

+333
-29
lines changed

‎src/libraries/System.Security.Cryptography.Cose/src/System/Security/Cryptography/Cose/CoseMessage.cs‎

Lines changed: 47 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -295,20 +295,43 @@ private static void DecodeUnprotectedBucket(CborReader reader, CoseHeaderMap hea
295295

296296
private static void DecodeBucket(CborReader reader, CoseHeaderMap headerParameters)
297297
{
298-
int? length = reader.ReadStartMap();
299-
for (int i = 0; i < length; i++)
298+
reader.ReadStartMap();
299+
300+
while (true)
300301
{
301-
CoseHeaderLabel label = reader.PeekState() switch
302+
CborReaderState state = reader.PeekState();
303+
304+
if (state == CborReaderState.EndMap)
305+
{
306+
reader.ReadEndMap();
307+
break;
308+
}
309+
310+
CoseHeaderLabel label = state switch
302311
{
303312
CborReaderState.UnsignedInteger or CborReaderState.NegativeInteger => new CoseHeaderLabel(reader.ReadInt32()),
304313
CborReaderState.TextString => new CoseHeaderLabel(reader.ReadTextString()),
305314
_ => throw new CryptographicException(SR.Format(SR.DecodeErrorWhileDecoding, SR.DecodeSign1MapLabelWasIncorrect))
306315
};
307316

308317
CoseHeaderValue value = CoseHeaderValue.FromEncodedValue(reader.ReadEncodedValue().Span);
309-
headerParameters.Add(label, value);
318+
319+
try
320+
{
321+
headerParameters.Add(label, value);
322+
}
323+
catch (ArgumentException e)
324+
{
325+
// Lift the well-known header value validation into a CryptographicException.
326+
if (e.ParamName == "value")
327+
{
328+
throw new CryptographicException(e.Message);
329+
}
330+
331+
Debug.Fail("Unexpected ArgumentException from CoseHeaderMap.Add");
332+
throw new CryptographicException(SR.DecodeErrorWhileDecodingSeeInnerEx, e);
333+
}
310334
}
311-
reader.ReadEndMap();
312335
}
313336

314337
private static byte[]? DecodePayload(CborReader reader)
@@ -563,13 +586,23 @@ internal static bool MissingCriticalHeaders(CoseHeaderMap? protectedHeders, out
563586
return false;
564587
}
565588

589+
bool empty = true;
590+
566591
var reader = new CborReader(critHeaderValue.EncodedValue);
567-
int length = reader.ReadStartArray().GetValueOrDefault();
568-
Debug.Assert(length > 0);
592+
reader.ReadStartArray();
569593

570-
for (int i = 0; i < length; i++)
594+
while (true)
571595
{
572-
CoseHeaderLabel label = reader.PeekState() switch
596+
CborReaderState state = reader.PeekState();
597+
598+
if (state == CborReaderState.EndArray)
599+
{
600+
reader.ReadEndArray();
601+
break;
602+
}
603+
604+
empty = false;
605+
CoseHeaderLabel label = state switch
573606
{
574607
CborReaderState.UnsignedInteger or CborReaderState.NegativeInteger => new CoseHeaderLabel(reader.ReadInt32()),
575608
CborReaderState.TextString => new CoseHeaderLabel(reader.ReadTextString()),
@@ -583,6 +616,11 @@ internal static bool MissingCriticalHeaders(CoseHeaderMap? protectedHeders, out
583616
}
584617
}
585618

619+
if (empty)
620+
{
621+
throw new CryptographicException(SR.CriticalHeadersMustBeArrayOfAtLeastOne);
622+
}
623+
586624
labelName = null;
587625
return false;
588626
}

‎src/libraries/System.Security.Cryptography.Cose/tests/CoseMessageTests.DecodeMultiSign.cs‎

Lines changed: 126 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -144,5 +144,131 @@ public void DecodeMultiSign_IndefiniteLengthArray_ShorterByOne(string hexCborPay
144144
CryptographicException ex = Assert.Throws<CryptographicException>(() => CoseMessage.DecodeMultiSign(cborPayload));
145145
Assert.Null(ex.InnerException);
146146
}
147+
148+
[Theory]
149+
[InlineData(true, true)]
150+
[InlineData(true, false)]
151+
[InlineData(false, false)]
152+
[InlineData(false, true)]
153+
public void DecodeMultiSignThrowsIfCriticalHeaderIsMissing(bool detached, bool useIndefiniteLength)
154+
{
155+
const string AttachedDefiniteHex =
156+
"D8628440A054546869732069732074686520636F6E74656E742E818347A20126" +
157+
"0281182AA05840ECB8C39BE15156FB6567C33634C75396D7FE1042C84FE54B9C" +
158+
"EFA51E674C0CB227A8C08E558B6047668BBE3311749776670D1583A14B3A2DD8" +
159+
"7F63F0FA298452";
160+
161+
const string AttachedIndefiniteHex =
162+
"D8628440A054546869732069732074686520636F6E74656E742E818348A20126" +
163+
"029F182AFFA05840F62CB760AC27D393D88ED392D5D4D55A02B0BB75261E75FE" +
164+
"9B346C280DA6B93BE7F5B1B66B74561513EA52CAA2C66FE7474010035C678DA6" +
165+
"B3549D3E671166EB";
166+
167+
const string DetachedDefiniteHex =
168+
"D8628440A0F6818347A201260281182AA05840F96CE3D0999F34BE0E3FC62AE2" +
169+
"AB25DD8D88F7154E6FADD5FFFEAF78F89DB97AC3E599ADB555C8442BD520F3F4" +
170+
"8CB6A320B864677E26D1FA79FEDD79C3BCA927";
171+
172+
const string DetachedIndefiniteHex =
173+
"D8628440A0F6818348A20126029F182AFFA0584028E95F7F9267CED0061339A7" +
174+
"6602D823774EDA3E8D53B0A4FA436B71B0DBCA6F03F561A67355374AF494648C" +
175+
"941558146F9C22B17542EBAF23497D27635A1829";
176+
177+
string inputHex = (detached, useIndefiniteLength) switch
178+
{
179+
(false, false) => AttachedDefiniteHex,
180+
(false, true) => AttachedIndefiniteHex,
181+
(true, false) => DetachedDefiniteHex,
182+
(true, true) => DetachedIndefiniteHex,
183+
};
184+
185+
AssertExtensions.ThrowsContains<CryptographicException>(
186+
() => CoseMessage.DecodeMultiSign(ByteUtils.HexToByteArray(inputHex)),
187+
"Critical Header '42' missing from protected map.");
188+
}
189+
190+
[Theory]
191+
[InlineData(true, true)]
192+
[InlineData(true, false)]
193+
[InlineData(false, false)]
194+
[InlineData(false, true)]
195+
public void DecodeMultiSignThrowsIfCriticalHeadersIsEmpty(bool detached, bool useIndefiniteLength)
196+
{
197+
const string AttachedDefiniteHex =
198+
"D8628440A054546869732069732074686520636F6E74656E742E818345A20126" +
199+
"0280A05840B5F9E21078643A74B181ED294AC72C71F20AC5CA7AD037F559C68E" +
200+
"06148429396A4194133763AB6918D747ACEE820CC430C2E891E3E2D5EECF6126" +
201+
"1CEA33C6D4";
202+
203+
const string AttachedIndefiniteHex =
204+
"D8628440A054546869732069732074686520636F6E74656E742E818346A20126" +
205+
"029FFFA05840DDF3C0B85415AD1628C0B50C0F3FEDE675C1003484687CDFA3FA" +
206+
"09285D5A31D48ADF11744BE0AE87F0189408A9CF38F0572537E8A786D505B6A6" +
207+
"EE2008B91C74";
208+
209+
const string DetachedDefiniteHex =
210+
"D8628440A0F6818345A201260280A05840EB66EE9E064CAB2E2F50244661734D" +
211+
"9AEBD959BD21278E8D4827870DFE10C27B52E3E21D29185FC64526DC3B80C108" +
212+
"548E956E9DBDDC7B23D100C17715AEE163";
213+
214+
const string DetachedIndefiniteHex =
215+
"D8628440A0F6818346A20126029FFFA05840FC954ABD1611F7C6EEDD7FE71C3F" +
216+
"62821AD46ED1988500F3309D0C607F0F151A69D0FC7BC968B2C36AEE68AC2B9A" +
217+
"9580DFE1244F6E5F834183497F21EA5900C1";
218+
219+
string inputHex = (detached, useIndefiniteLength) switch
220+
{
221+
(false, false) => AttachedDefiniteHex,
222+
(false, true) => AttachedIndefiniteHex,
223+
(true, false) => DetachedDefiniteHex,
224+
(true, true) => DetachedIndefiniteHex,
225+
};
226+
227+
AssertExtensions.ThrowsContains<CryptographicException>(
228+
() => CoseMessage.DecodeMultiSign(ByteUtils.HexToByteArray(inputHex)),
229+
"Critical Headers must be a CBOR array of at least one element.");
230+
}
231+
232+
[Theory]
233+
[InlineData(true, true)]
234+
[InlineData(true, false)]
235+
[InlineData(false, false)]
236+
[InlineData(false, true)]
237+
public void DecodeMultiSignThrowsIfCriticalHeaderIsOfUnknownType(bool detached, bool useIndefiniteLength)
238+
{
239+
const string AttachedDefiniteHex =
240+
"D8628440A054546869732069732074686520636F6E74656E742E818347A20126" +
241+
"0281412AA05840FCAFEDBE41693C7BA43FB58E2CF06182BE1BF340122CC5AFD4" +
242+
"F59172C7E95166FF8E98FE9A0C2BEFEA135FD800DE6CA9A281D49B141CB93B17" +
243+
"D992E693540F8A";
244+
245+
const string AttachedIndefiniteHex =
246+
"D8628440A054546869732069732074686520636F6E74656E742E818348A20126" +
247+
"029F412AFFA058400D3F4426B26007D731677D99B542E524847FF3927BCA74E4" +
248+
"1823B09D6CA57A0E107F93DFE5DB851F4CEE8C0E4AF83E3540848F026FCD761F" +
249+
"91CA2ED8D5F98134";
250+
251+
const string DetachedDefiniteHex =
252+
"D8628440A0F6818347A201260281412AA0584008E0EEF66622FEC926CB651E90" +
253+
"13D8628AB72581533761EDE52972FE6DFBF2C4BADB6C218E8AD1E28F8192DFB2" +
254+
"8A82A4444A74C370AEA6C63AC982EABCD52874";
255+
256+
const string DetachedIndefiniteHex =
257+
"D8628440A0F6818348A20126029F412AFFA05840C6DDCA2F35B7B285AB594963" +
258+
"E9DB43CBDC77842256A7D1D31704749C7446AD5A67BBC02F9DBAF8F394ECCCA7" +
259+
"8E8B63E5BB746F0205EE5732DFB2E00EBA3D5F48";
260+
261+
string inputHex = (detached, useIndefiniteLength) switch
262+
{
263+
(false, false) => AttachedDefiniteHex,
264+
(false, true) => AttachedIndefiniteHex,
265+
(true, false) => DetachedDefiniteHex,
266+
(true, true) => DetachedIndefiniteHex,
267+
};
268+
269+
AssertExtensions.ThrowsContains<CryptographicException>(
270+
() => CoseMessage.DecodeMultiSign(ByteUtils.HexToByteArray(inputHex)),
271+
"Header '2' does not accept the specified value.");
272+
}
147273
}
148274
}

‎src/libraries/System.Security.Cryptography.Cose/tests/CoseMessageTests.DecodeSign1.cs‎

Lines changed: 125 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -102,5 +102,130 @@ public void DecodeSign1_IndefiniteLengthArray_ShorterByOne()
102102
CryptographicException ex = Assert.Throws<CryptographicException>(() => CoseMessage.DecodeSign1(cborPayload));
103103
Assert.Null(ex.InnerException);
104104
}
105+
106+
[Theory]
107+
[InlineData(true, true)]
108+
[InlineData(true, false)]
109+
[InlineData(false, false)]
110+
[InlineData(false, true)]
111+
public void DecodeSign1ThrowsIfCriticalHeaderIsMissing(bool detached, bool useIndefiniteLength)
112+
{
113+
const string AttachedDefiniteHex =
114+
"D28447A201260281182AA054546869732069732074686520636F6E74656E742E" +
115+
"5840F78745BDFA8CDF90ED6EC130BC8D97F43C8A52899920221832A1E758A1E7" +
116+
"590827148F6D1A76673E7E9615F628730B19F07707B6FB1C9CD7B6D4E2B3C3F0" +
117+
"DEAD";
118+
119+
const string AttachedIndefiniteHex =
120+
"D28448A20126029F182AFFA054546869732069732074686520636F6E74656E74" +
121+
"2E58408B07F60298F64453356EAF005C630A4576AF4C66E0327579BB81B5D726" +
122+
"3836AA9419B1312298DD47BC10BA22D6DEEE35F1526948BF098915816149B46A" +
123+
"3C9981";
124+
125+
const string DetachedDefiniteHex =
126+
"D28447A201260281182AA0F6584089B093A038B0636940F9273EF11214B64CC1" +
127+
"BB862305EDEC9C772A3D5089A54A6CBBA00323FA59A593A828F157653DEE15B0" +
128+
"EBBDC070D02CDFD13E8A9F2ECA1B";
129+
130+
const string DetachedIndefiniteHex =
131+
"D28448A20126029F182AFFA0F658409B35B9FD294BDF36EEF7494D0EC9E19F6A2" +
132+
"106638FD4A2A31B816FED80493772DCEA8B64F6618119E278379F83E1A62BA382" +
133+
"21B9F1AC705FAD8612DC6B0478A0";
134+
135+
string inputHex = (detached, useIndefiniteLength) switch
136+
{
137+
(false, false) => AttachedDefiniteHex,
138+
(false, true) => AttachedIndefiniteHex,
139+
(true, false) => DetachedDefiniteHex,
140+
(true, true) => DetachedIndefiniteHex,
141+
};
142+
143+
AssertExtensions.ThrowsContains<CryptographicException>(
144+
() => CoseMessage.DecodeSign1(ByteUtils.HexToByteArray(inputHex)),
145+
"Critical Header '42' missing from protected map.");
146+
}
147+
148+
[Theory]
149+
[InlineData(true, true)]
150+
[InlineData(true, false)]
151+
[InlineData(false, false)]
152+
[InlineData(false, true)]
153+
public void DecodeSign1ThrowsIfCriticalHeadersIsEmpty(bool detached, bool useIndefiniteLength)
154+
{
155+
const string AttachedDefiniteHex =
156+
"D28445A201260280A054546869732069732074686520636F6E74656E742E5840" +
157+
"57C7EE86AF06B1ABB002480CE148DFDA06C2CA4AFE83E9C7AE3493EA13E06E9B" +
158+
"0A4C713F7FDCDD2F8731103CDA28B83313E411988B88AC7716E43307B5AF22FD";
159+
160+
const string AttachedIndefiniteHex =
161+
"D28446A20126029FFFA054546869732069732074686520636F6E74656E742E58" +
162+
"401B941A9C799270827BE5139EC5F3DE4E072913F6473C7278E691D6C58D407A" +
163+
"23DB3176383E8429AA558418EE33CB7DFFD2CF251EEC93B6CFC300D0D9679CE5" +
164+
"42";
165+
166+
const string DetachedDefiniteHex =
167+
"D28445A201260280A0F658409B0EBC937A969A7D4BB2AA0B1004091EDAA00AE2" +
168+
"BBCCBB994B7278C9E50C6C734B3A53CB5B87A99E75F63D16B73757CA23C99CF0" +
169+
"8F8F909A1332DAC05D9DB1C0";
170+
171+
const string DetachedIndefiniteHex =
172+
"D28446A20126029FFFA0F65840CA96F1292FEE2B787DC75D91553024E70DD62B" +
173+
"EA0BFE284024385C6D9493EEF6F055825E79244B63E76F69A419C3A36B3B1F18" +
174+
"34789A23983D685B7CDA231E86";
175+
176+
string inputHex = (detached, useIndefiniteLength) switch
177+
{
178+
(false, false) => AttachedDefiniteHex,
179+
(false, true) => AttachedIndefiniteHex,
180+
(true, false) => DetachedDefiniteHex,
181+
(true, true) => DetachedIndefiniteHex,
182+
};
183+
184+
AssertExtensions.ThrowsContains<CryptographicException>(
185+
() => CoseMessage.DecodeSign1(ByteUtils.HexToByteArray(inputHex)),
186+
"Critical Headers must be a CBOR array of at least one element.");
187+
}
188+
189+
[Theory]
190+
[InlineData(true, true)]
191+
[InlineData(true, false)]
192+
[InlineData(false, false)]
193+
[InlineData(false, true)]
194+
public void DecodeSign1ThrowsIfCriticalHeaderIsOfUnknownType(bool detached, bool useIndefiniteLength)
195+
{
196+
const string AttachedDefiniteHex =
197+
"D28447A201260281412AA054546869732069732074686520636F6E74656E742E" +
198+
"58403529AC69F69A80B4055CFFCA88F010390509E0A9D4D0083F23DF46841144" +
199+
"B7E9D7CC11E90D0D51103672083449B439B71EAF6B922C011CC471D8E1D577C6" +
200+
"B954";
201+
202+
const string AttachedIndefiniteHex =
203+
"D28448A20126029F412AFFA054546869732069732074686520636F6E74656E74" +
204+
"2E5840FE8A2CBBBA2A154361BEF0892D11FF621A1DBDCBD1A955020DD7D85ED8" +
205+
"15C43B3AB39A32561AAEF679D08FD561339AC9A4E537B2E91DC120A32F406455" +
206+
"F3353F";
207+
208+
const string DetachedDefiniteHex =
209+
"D28447A201260281412AA0F65840AB87DA5ABA5A470C7508F5F1724744458407" +
210+
"897746890428F877AD593F9D90E5503A6D1B3369AF77952223D5C474CBB8EC62" +
211+
"9726F967921A4AB91DC8F86DA1CF";
212+
213+
const string DetachedIndefiniteHex =
214+
"D28448A20126029F412AFFA0F658409613065203B619BE9CEC1CC596F59C7395" +
215+
"5AEE8BD492F16B72D2C0F443AE70E5E5B1D615A06A90145078B41A1CA12D4067" +
216+
"D6C6CEEB2C19B3747A0926305EBA09";
217+
218+
string inputHex = (detached, useIndefiniteLength) switch
219+
{
220+
(false, false) => AttachedDefiniteHex,
221+
(false, true) => AttachedIndefiniteHex,
222+
(true, false) => DetachedDefiniteHex,
223+
(true, true) => DetachedIndefiniteHex,
224+
};
225+
226+
AssertExtensions.ThrowsContains<CryptographicException>(
227+
() => CoseMessage.DecodeSign1(ByteUtils.HexToByteArray(inputHex)),
228+
"Header '2' does not accept the specified value.");
229+
}
105230
}
106231
}

0 commit comments

Comments
 (0)