Allow ExtensionObjects which are encoded with an unknown size (#2869)

mregen · web-flow · commit a887f909f1d3 · 2024-12-03T12:06:11.000+01:00
An older OPC UA server returns well known extension object(811) which cannot be decoded by the client because the server doesn't set the length of the extension object. 
Recently, based on fuzz testing, the length check has been changed to enforce an exact match, according to spec.

Workaround: Allow a value of -1 for the length, then allow to decode well known extension objects.

Also improve some internal calls to Safe Read functions by calling not via the interface.
diff --git a/Stack/Opc.Ua.Core/Types/Encoders/BinaryDecoder.cs b/Stack/Opc.Ua.Core/Types/Encoders/BinaryDecoder.cs
@@ -901,7 +901,7 @@ public Int32Collection ReadInt32Array(string fieldName)
 
             for (int ii = 0; ii < length; ii++)
             {
-                values.Add(ReadInt32(null));
+                values.Add(SafeReadInt32());
             }
 
             return values;
@@ -945,7 +945,7 @@ public Int64Collection ReadInt64Array(string fieldName)
 
             for (int ii = 0; ii < length; ii++)
             {
-                values.Add(ReadInt64(null));
+                values.Add(SafeReadInt64());
             }
 
             return values;
@@ -967,7 +967,7 @@ public UInt64Collection ReadUInt64Array(string fieldName)
 
             for (int ii = 0; ii < length; ii++)
             {
-                values.Add(ReadUInt64(null));
+                values.Add(SafeReadUInt64());
             }
 
             return values;
@@ -989,7 +989,7 @@ public FloatCollection ReadFloatArray(string fieldName)
 
             for (int ii = 0; ii < length; ii++)
             {
-                values.Add(ReadFloat(null));
+                values.Add(SafeReadFloat());
             }
 
             return values;
@@ -1011,7 +1011,7 @@ public DoubleCollection ReadDoubleArray(string fieldName)
 
             for (int ii = 0; ii < length; ii++)
             {
-                values.Add(ReadDouble(null));
+                values.Add(SafeReadDouble());
             }
 
             return values;
@@ -1547,22 +1547,22 @@ private DiagnosticInfo ReadDiagnosticInfo(string fieldName, int depth)
                 // read the fields of the diagnostic info structure.
                 if ((encodingByte & (byte)DiagnosticInfoEncodingBits.SymbolicId) != 0)
                 {
-                    value.SymbolicId = ReadInt32(null);
+                    value.SymbolicId = SafeReadInt32();
                 }
 
                 if ((encodingByte & (byte)DiagnosticInfoEncodingBits.NamespaceUri) != 0)
                 {
-                    value.NamespaceUri = ReadInt32(null);
+                    value.NamespaceUri = SafeReadInt32();
                 }
 
                 if ((encodingByte & (byte)DiagnosticInfoEncodingBits.Locale) != 0)
                 {
-                    value.Locale = ReadInt32(null);
+                    value.Locale = SafeReadInt32();
                 }
 
                 if ((encodingByte & (byte)DiagnosticInfoEncodingBits.LocalizedText) != 0)
                 {
-                    value.LocalizedText = ReadInt32(null);
+                    value.LocalizedText = SafeReadInt32();
                 }
 
                 if ((encodingByte & (byte)DiagnosticInfoEncodingBits.AdditionalInfo) != 0)
@@ -1617,7 +1617,7 @@ private Array ReadArrayElements(int length, BuiltInType builtInType)
 
                     for (int ii = 0; ii < values.Length; ii++)
                     {
-                        values[ii] = ReadBoolean(null);
+                        values[ii] = SafeReadBoolean();
                     }
 
                     array = values;
@@ -1656,7 +1656,7 @@ private Array ReadArrayElements(int length, BuiltInType builtInType)
 
                     for (int ii = 0; ii < values.Length; ii++)
                     {
-                        values[ii] = ReadInt16(null);
+                        values[ii] = SafeReadInt16();
                     }
 
                     array = values;
@@ -1669,7 +1669,7 @@ private Array ReadArrayElements(int length, BuiltInType builtInType)
 
                     for (int ii = 0; ii < values.Length; ii++)
                     {
-                        values[ii] = ReadUInt16(null);
+                        values[ii] = SafeReadUInt16();
                     }
 
                     array = values;
@@ -1683,7 +1683,7 @@ private Array ReadArrayElements(int length, BuiltInType builtInType)
 
                     for (int ii = 0; ii < values.Length; ii++)
                     {
-                        values[ii] = ReadInt32(null);
+                        values[ii] = SafeReadInt32();
                     }
                     array = values;
                     break;
@@ -1708,7 +1708,7 @@ private Array ReadArrayElements(int length, BuiltInType builtInType)
 
                     for (int ii = 0; ii < values.Length; ii++)
                     {
-                        values[ii] = ReadInt64(null);
+                        values[ii] = SafeReadInt64();
                     }
 
                     array = values;
@@ -1721,7 +1721,7 @@ private Array ReadArrayElements(int length, BuiltInType builtInType)
 
                     for (int ii = 0; ii < values.Length; ii++)
                     {
-                        values[ii] = ReadUInt64(null);
+                        values[ii] = SafeReadUInt64();
                     }
 
                     array = values;
@@ -1734,7 +1734,7 @@ private Array ReadArrayElements(int length, BuiltInType builtInType)
 
                     for (int ii = 0; ii < values.Length; ii++)
                     {
-                        values[ii] = ReadFloat(null);
+                        values[ii] = SafeReadFloat();
                     }
 
                     array = values;
@@ -1747,7 +1747,7 @@ private Array ReadArrayElements(int length, BuiltInType builtInType)
 
                     for (int ii = 0; ii < values.Length; ii++)
                     {
-                        values[ii] = ReadDouble(null);
+                        values[ii] = SafeReadDouble();
                     }
 
                     array = values;
@@ -2096,15 +2096,16 @@ private ExtensionObject ReadExtensionObject()
                 return extension;
             }
 
-            // get the length.
-            int length = ReadInt32(null);
+            // Get the length.
+            // Allow a length of -1 to support legacy devices that don't fill the length correctly
+            int length = SafeReadInt32();
 
             // save the current position.
             int start = Position;
 
             // create instance of type.
             IEncodeable encodeable = null;
-            if (systemType != null && length >= 0)
+            if (systemType != null && length >= -1)
             {
                 encodeable = Activator.CreateInstance(systemType) as IEncodeable;
 
@@ -2132,7 +2133,7 @@ private ExtensionObject ReadExtensionObject()
 
                     // verify the decoder did not exceed the length of the encodeable object
                     int used = Position - start;
-                    if (length != used)
+                    if (length >= 0 && length != used)
                     {
                         errorMessage = "Length mismatch";
                         exception = null;
@@ -2211,11 +2212,14 @@ private ExtensionObject ReadExtensionObject()
             }
 
             // any unread data indicates a decoding error.
-            long unused = length - (Position - start);
-            if (unused > 0)
+            if (length >= 0)
             {
-                throw ServiceResultException.Create(StatusCodes.BadDecodingError,
-                    "Cannot skip {0} bytes of unknown extension object body with type '{1}'.", unused, extension.TypeId);
+                long unused = length - (Position - start);
+                if (unused > 0)
+                {
+                    throw ServiceResultException.Create(StatusCodes.BadDecodingError,
+                        "Cannot skip {0} bytes of unknown extension object body with type '{1}'.", unused, extension.TypeId);
+                }
             }
 
             if (encodeable != null)
@@ -2307,7 +2311,7 @@ private Variant ReadVariantValue(string fieldName)
 
                     case BuiltInType.Boolean:
                     {
-                        value.Set(ReadBoolean(null));
+                        value.Set(SafeReadBoolean());
                         break;
                     }
 
@@ -2325,20 +2329,20 @@ private Variant ReadVariantValue(string fieldName)
 
                     case BuiltInType.Int16:
                     {
-                        value.Set(ReadInt16(null));
+                        value.Set(SafeReadInt16());
                         break;
                     }
 
                     case BuiltInType.UInt16:
                     {
-                        value.Set(ReadUInt16(null));
+                        value.Set(SafeReadUInt16());
                         break;
                     }
 
                     case BuiltInType.Int32:
                     case BuiltInType.Enumeration:
                     {
-                        value.Set(ReadInt32(null));
+                        value.Set(SafeReadInt32());
                         break;
                     }
 
@@ -2350,25 +2354,25 @@ private Variant ReadVariantValue(string fieldName)
 
                     case BuiltInType.Int64:
                     {
-                        value.Set(ReadInt64(null));
+                        value.Set(SafeReadInt64());
                         break;
                     }
 
                     case BuiltInType.UInt64:
                     {
-                        value.Set(ReadUInt64(null));
+                        value.Set(SafeReadUInt64());
                         break;
                     }
 
                     case BuiltInType.Float:
                     {
-                        value.Set(ReadFloat(null));
+                        value.Set(SafeReadFloat());
                         break;
                     }
 
                     case BuiltInType.Double:
                     {
-                        value.Set(ReadDouble(null));
+                        value.Set(SafeReadDouble());
                         break;
                     }
 

Original file line number	Diff line number	Diff line change
`@@ -901,7 +901,7 @@ public Int32Collection ReadInt32Array(string fieldName)`
`901`	`901`
`902`	`902`	`for (int ii = 0; ii < length; ii++)`
`903`	`903`	`{`
`904`		`- values.Add(ReadInt32(null));`
	`904`	`+ values.Add(SafeReadInt32());`
`905`	`905`	`}`
`906`	`906`
`907`	`907`	`return values;`
`@@ -945,7 +945,7 @@ public Int64Collection ReadInt64Array(string fieldName)`
`945`	`945`
`946`	`946`	`for (int ii = 0; ii < length; ii++)`
`947`	`947`	`{`
`948`		`- values.Add(ReadInt64(null));`
	`948`	`+ values.Add(SafeReadInt64());`
`949`	`949`	`}`
`950`	`950`
`951`	`951`	`return values;`
`@@ -967,7 +967,7 @@ public UInt64Collection ReadUInt64Array(string fieldName)`
`967`	`967`
`968`	`968`	`for (int ii = 0; ii < length; ii++)`
`969`	`969`	`{`
`970`		`- values.Add(ReadUInt64(null));`
	`970`	`+ values.Add(SafeReadUInt64());`
`971`	`971`	`}`
`972`	`972`
`973`	`973`	`return values;`
`@@ -989,7 +989,7 @@ public FloatCollection ReadFloatArray(string fieldName)`
`989`	`989`
`990`	`990`	`for (int ii = 0; ii < length; ii++)`
`991`	`991`	`{`
`992`		`- values.Add(ReadFloat(null));`
	`992`	`+ values.Add(SafeReadFloat());`
`993`	`993`	`}`
`994`	`994`
`995`	`995`	`return values;`
`@@ -1011,7 +1011,7 @@ public DoubleCollection ReadDoubleArray(string fieldName)`
`1011`	`1011`
`1012`	`1012`	`for (int ii = 0; ii < length; ii++)`
`1013`	`1013`	`{`
`1014`		`- values.Add(ReadDouble(null));`
	`1014`	`+ values.Add(SafeReadDouble());`
`1015`	`1015`	`}`
`1016`	`1016`
`1017`	`1017`	`return values;`
`@@ -1547,22 +1547,22 @@ private DiagnosticInfo ReadDiagnosticInfo(string fieldName, int depth)`
`1547`	`1547`	`// read the fields of the diagnostic info structure.`
`1548`	`1548`	`if ((encodingByte & (byte)DiagnosticInfoEncodingBits.SymbolicId) != 0)`
`1549`	`1549`	`{`
`1550`		`- value.SymbolicId = ReadInt32(null);`
	`1550`	`+ value.SymbolicId = SafeReadInt32();`
`1551`	`1551`	`}`
`1552`	`1552`
`1553`	`1553`	`if ((encodingByte & (byte)DiagnosticInfoEncodingBits.NamespaceUri) != 0)`
`1554`	`1554`	`{`
`1555`		`- value.NamespaceUri = ReadInt32(null);`
	`1555`	`+ value.NamespaceUri = SafeReadInt32();`
`1556`	`1556`	`}`
`1557`	`1557`
`1558`	`1558`	`if ((encodingByte & (byte)DiagnosticInfoEncodingBits.Locale) != 0)`
`1559`	`1559`	`{`
`1560`		`- value.Locale = ReadInt32(null);`
	`1560`	`+ value.Locale = SafeReadInt32();`
`1561`	`1561`	`}`
`1562`	`1562`
`1563`	`1563`	`if ((encodingByte & (byte)DiagnosticInfoEncodingBits.LocalizedText) != 0)`
`1564`	`1564`	`{`
`1565`		`- value.LocalizedText = ReadInt32(null);`
	`1565`	`+ value.LocalizedText = SafeReadInt32();`
`1566`	`1566`	`}`
`1567`	`1567`
`1568`	`1568`	`if ((encodingByte & (byte)DiagnosticInfoEncodingBits.AdditionalInfo) != 0)`
`@@ -1617,7 +1617,7 @@ private Array ReadArrayElements(int length, BuiltInType builtInType)`
`1617`	`1617`
`1618`	`1618`	`for (int ii = 0; ii < values.Length; ii++)`
`1619`	`1619`	`{`
`1620`		`- values[ii] = ReadBoolean(null);`
	`1620`	`+ values[ii] = SafeReadBoolean();`
`1621`	`1621`	`}`
`1622`	`1622`
`1623`	`1623`	`array = values;`
`@@ -1656,7 +1656,7 @@ private Array ReadArrayElements(int length, BuiltInType builtInType)`
`1656`	`1656`
`1657`	`1657`	`for (int ii = 0; ii < values.Length; ii++)`
`1658`	`1658`	`{`
`1659`		`- values[ii] = ReadInt16(null);`
	`1659`	`+ values[ii] = SafeReadInt16();`
`1660`	`1660`	`}`
`1661`	`1661`
`1662`	`1662`	`array = values;`
`@@ -1669,7 +1669,7 @@ private Array ReadArrayElements(int length, BuiltInType builtInType)`
`1669`	`1669`
`1670`	`1670`	`for (int ii = 0; ii < values.Length; ii++)`
`1671`	`1671`	`{`
`1672`		`- values[ii] = ReadUInt16(null);`
	`1672`	`+ values[ii] = SafeReadUInt16();`
`1673`	`1673`	`}`
`1674`	`1674`
`1675`	`1675`	`array = values;`
`@@ -1683,7 +1683,7 @@ private Array ReadArrayElements(int length, BuiltInType builtInType)`
`1683`	`1683`
`1684`	`1684`	`for (int ii = 0; ii < values.Length; ii++)`
`1685`	`1685`	`{`
`1686`		`- values[ii] = ReadInt32(null);`
	`1686`	`+ values[ii] = SafeReadInt32();`
`1687`	`1687`	`}`
`1688`	`1688`	`array = values;`
`1689`	`1689`	`break;`
`@@ -1708,7 +1708,7 @@ private Array ReadArrayElements(int length, BuiltInType builtInType)`
`1708`	`1708`
`1709`	`1709`	`for (int ii = 0; ii < values.Length; ii++)`
`1710`	`1710`	`{`
`1711`		`- values[ii] = ReadInt64(null);`
	`1711`	`+ values[ii] = SafeReadInt64();`
`1712`	`1712`	`}`
`1713`	`1713`
`1714`	`1714`	`array = values;`
`@@ -1721,7 +1721,7 @@ private Array ReadArrayElements(int length, BuiltInType builtInType)`
`1721`	`1721`
`1722`	`1722`	`for (int ii = 0; ii < values.Length; ii++)`
`1723`	`1723`	`{`
`1724`		`- values[ii] = ReadUInt64(null);`
	`1724`	`+ values[ii] = SafeReadUInt64();`
`1725`	`1725`	`}`
`1726`	`1726`
`1727`	`1727`	`array = values;`
`@@ -1734,7 +1734,7 @@ private Array ReadArrayElements(int length, BuiltInType builtInType)`
`1734`	`1734`
`1735`	`1735`	`for (int ii = 0; ii < values.Length; ii++)`
`1736`	`1736`	`{`
`1737`		`- values[ii] = ReadFloat(null);`
	`1737`	`+ values[ii] = SafeReadFloat();`
`1738`	`1738`	`}`
`1739`	`1739`
`1740`	`1740`	`array = values;`
`@@ -1747,7 +1747,7 @@ private Array ReadArrayElements(int length, BuiltInType builtInType)`
`1747`	`1747`
`1748`	`1748`	`for (int ii = 0; ii < values.Length; ii++)`
`1749`	`1749`	`{`
`1750`		`- values[ii] = ReadDouble(null);`
	`1750`	`+ values[ii] = SafeReadDouble();`
`1751`	`1751`	`}`
`1752`	`1752`
`1753`	`1753`	`array = values;`
`@@ -2096,15 +2096,16 @@ private ExtensionObject ReadExtensionObject()`
`2096`	`2096`	`return extension;`
`2097`	`2097`	`}`
`2098`	`2098`
`2099`		`- // get the length.`
`2100`		`- int length = ReadInt32(null);`
	`2099`	`+ // Get the length.`
	`2100`	`+ // Allow a length of -1 to support legacy devices that don't fill the length correctly`
	`2101`	`+ int length = SafeReadInt32();`
`2101`	`2102`
`2102`	`2103`	`// save the current position.`
`2103`	`2104`	`int start = Position;`
`2104`	`2105`
`2105`	`2106`	`// create instance of type.`
`2106`	`2107`	`IEncodeable encodeable = null;`
`2107`		`- if (systemType != null && length >= 0)`
	`2108`	`+ if (systemType != null && length >= -1)`
`2108`	`2109`	`{`
`2109`	`2110`	`encodeable = Activator.CreateInstance(systemType) as IEncodeable;`
`2110`	`2111`
`@@ -2132,7 +2133,7 @@ private ExtensionObject ReadExtensionObject()`
`2132`	`2133`
`2133`	`2134`	`// verify the decoder did not exceed the length of the encodeable object`
`2134`	`2135`	`int used = Position - start;`
`2135`		`- if (length != used)`
	`2136`	`+ if (length >= 0 && length != used)`
`2136`	`2137`	`{`
`2137`	`2138`	`errorMessage = "Length mismatch";`
`2138`	`2139`	`exception = null;`
`@@ -2211,11 +2212,14 @@ private ExtensionObject ReadExtensionObject()`
`2211`	`2212`	`}`
`2212`	`2213`
`2213`	`2214`	`// any unread data indicates a decoding error.`
`2214`		`- long unused = length - (Position - start);`
`2215`		`- if (unused > 0)`
	`2215`	`+ if (length >= 0)`
`2216`	`2216`	`{`
`2217`		`- throw ServiceResultException.Create(StatusCodes.BadDecodingError,`
`2218`		`- "Cannot skip {0} bytes of unknown extension object body with type '{1}'.", unused, extension.TypeId);`
	`2217`	`+ long unused = length - (Position - start);`
	`2218`	`+ if (unused > 0)`
	`2219`	`+ {`
	`2220`	`+ throw ServiceResultException.Create(StatusCodes.BadDecodingError,`
	`2221`	`+ "Cannot skip {0} bytes of unknown extension object body with type '{1}'.", unused, extension.TypeId);`
	`2222`	`+ }`
`2219`	`2223`	`}`
`2220`	`2224`
`2221`	`2225`	`if (encodeable != null)`
`@@ -2307,7 +2311,7 @@ private Variant ReadVariantValue(string fieldName)`
`2307`	`2311`
`2308`	`2312`	`case BuiltInType.Boolean:`
`2309`	`2313`	`{`
`2310`		`- value.Set(ReadBoolean(null));`
	`2314`	`+ value.Set(SafeReadBoolean());`
`2311`	`2315`	`break;`
`2312`	`2316`	`}`
`2313`	`2317`
`@@ -2325,20 +2329,20 @@ private Variant ReadVariantValue(string fieldName)`
`2325`	`2329`
`2326`	`2330`	`case BuiltInType.Int16:`
`2327`	`2331`	`{`
`2328`		`- value.Set(ReadInt16(null));`
	`2332`	`+ value.Set(SafeReadInt16());`
`2329`	`2333`	`break;`
`2330`	`2334`	`}`
`2331`	`2335`
`2332`	`2336`	`case BuiltInType.UInt16:`
`2333`	`2337`	`{`
`2334`		`- value.Set(ReadUInt16(null));`
	`2338`	`+ value.Set(SafeReadUInt16());`
`2335`	`2339`	`break;`
`2336`	`2340`	`}`
`2337`	`2341`
`2338`	`2342`	`case BuiltInType.Int32:`
`2339`	`2343`	`case BuiltInType.Enumeration:`
`2340`	`2344`	`{`
`2341`		`- value.Set(ReadInt32(null));`
	`2345`	`+ value.Set(SafeReadInt32());`
`2342`	`2346`	`break;`
`2343`	`2347`	`}`
`2344`	`2348`
`@@ -2350,25 +2354,25 @@ private Variant ReadVariantValue(string fieldName)`
`2350`	`2354`
`2351`	`2355`	`case BuiltInType.Int64:`
`2352`	`2356`	`{`
`2353`		`- value.Set(ReadInt64(null));`
	`2357`	`+ value.Set(SafeReadInt64());`
`2354`	`2358`	`break;`
`2355`	`2359`	`}`
`2356`	`2360`
`2357`	`2361`	`case BuiltInType.UInt64:`
`2358`	`2362`	`{`
`2359`		`- value.Set(ReadUInt64(null));`
	`2363`	`+ value.Set(SafeReadUInt64());`
`2360`	`2364`	`break;`
`2361`	`2365`	`}`
`2362`	`2366`
`2363`	`2367`	`case BuiltInType.Float:`
`2364`	`2368`	`{`
`2365`		`- value.Set(ReadFloat(null));`
	`2369`	`+ value.Set(SafeReadFloat());`
`2366`	`2370`	`break;`
`2367`	`2371`	`}`
`2368`	`2372`
`2369`	`2373`	`case BuiltInType.Double:`
`2370`	`2374`	`{`
`2371`		`- value.Set(ReadDouble(null));`
	`2375`	`+ value.Set(SafeReadDouble());`
`2372`	`2376`	`break;`
`2373`	`2377`	`}`
`2374`	`2378`