Add max length of string reads (#371)

Added a maximum length to read from strings to prevent OOM/Overflow if there's corrupt memory. Fixes #244.
microsoft · Dec 4, 2019 · bea024c · bea024c
1 parent afc30c5
commit bea024c
Show file tree

Hide file tree

Showing 4 changed files with 13 additions and 16 deletions.
diff --git a/src/Microsoft.Diagnostics.Runtime/src/Common/ClrObject.cs b/src/Microsoft.Diagnostics.Runtime/src/Common/ClrObject.cs
@@ -118,13 +118,8 @@ public override string ToString()
         /// Converts a ClrObject into its string value.
         /// </summary>
         /// <param name="obj">A string object.</param>
-        public static explicit operator string(ClrObject obj)
-        {
-            if (!obj.Type.IsString)
-                throw new InvalidOperationException("Object {obj} is not a string.");
+        public static explicit operator string(ClrObject obj) => obj.AsString();
 
-            return ValueReader.GetStringContents(obj.Type, obj.Helpers.DataReader, obj.Address);
-        }
 
         /// <summary>
         /// Returns <see cref="Address"/> sweetening obj to pointer move.
@@ -231,7 +226,7 @@ public ClrType AsRuntimeType()
         /// </summary>
         /// <param name="fieldName">The name of the field to get the value for.</param>
         /// <returns>The value of the given field.</returns>
-        public string GetStringField(string fieldName)
+        public string GetStringField(string fieldName, int maxLength = 4096)
         {
             ulong address = GetFieldAddress(fieldName, ClrElementType.String, out ClrType stringType, "string");
             if (!Helpers.DataReader.ReadPointer(address, out ulong strPtr))
@@ -240,15 +235,16 @@ public string GetStringField(string fieldName)
             if (strPtr == 0)
                 return null;
 
-            return ValueReader.GetStringContents(stringType, Helpers.DataReader, strPtr);
+            return ValueReader.GetStringContents(stringType, Helpers.DataReader, strPtr, maxLength);
         }
 
-        public string AsString()
+        public string AsString(int maxLength = 4096)
         {
             if (!Type.IsString)
-                throw new InvalidOperationException();
+                throw new InvalidOperationException($"Object {Address:x} is not a string, actual type: {Type?.Name ?? "null"}.");
+
 
-            return ValueReader.GetStringContents(Type, Helpers.DataReader, Address);
+            return ValueReader.GetStringContents(Type, Helpers.DataReader, Address, maxLength);
         }
 
         private ulong GetFieldAddress(string fieldName, ClrElementType element, out ClrType fieldType, string typeName)

diff --git a/src/Microsoft.Diagnostics.Runtime/src/Common/ClrValueClass.cs b/src/Microsoft.Diagnostics.Runtime/src/Common/ClrValueClass.cs
@@ -108,7 +108,7 @@ public ClrValueClass GetValueClassField(string fieldName)
         /// </summary>
         /// <param name="fieldName">The name of the field to get the value for.</param>
         /// <returns>The value of the given field.</returns>
-        public string GetStringField(string fieldName)
+        public string GetStringField(string fieldName, int maxLength = 4096)
         {
             ulong address = GetFieldAddress(fieldName, ClrElementType.String, "string");
             if (!DataReader.ReadPointer(address, out ulong str))
@@ -118,7 +118,7 @@ public string GetStringField(string fieldName)
                 return null;
 
             ClrObject obj = new ClrObject(str, Type.Heap.StringType);
-            return obj.AsString();
+            return obj.AsString(maxLength);
         }
 
         private ulong GetFieldAddress(string fieldName, ClrElementType element, string typeName)

diff --git a/src/Microsoft.Diagnostics.Runtime/src/Common/IAddressableTypedEntity.cs b/src/Microsoft.Diagnostics.Runtime/src/Common/IAddressableTypedEntity.cs
@@ -41,7 +41,7 @@ public interface IAddressableTypedEntity : IEquatable<IAddressableTypedEntity>
         /// <exception cref="ArgumentException">Thrown when field was not found by name.</exception>
         /// <exception cref="InvalidOperationException">Thrown when found field has other type than <see cref="string"/>.</exception>
         /// <exception cref="MemoryReadException">Thrown when object reference could not be followed, or <see cref="string"/> could not be read.</exception>
-        string GetStringField(string fieldName);
+        string GetStringField(string fieldName, int maxLength = 4096);
 
         /// <summary>
         /// Gets the struct field value from the entity field.

diff --git a/src/Microsoft.Diagnostics.Runtime/src/Implementation/ValueReader.cs b/src/Microsoft.Diagnostics.Runtime/src/Implementation/ValueReader.cs
@@ -16,7 +16,7 @@ internal static object GetValueAtAddress(ClrHeap heap, IDataReader reader, ClrEl
             switch (cet)
             {
                 case ClrElementType.String:
-                    return GetStringContents(heap.StringType, reader, addr);
+                    return GetStringContents(heap.StringType, reader, addr, 4096);
 
                 case ClrElementType.Class:
                 case ClrElementType.Array:
@@ -138,7 +138,7 @@ internal static object GetValueAtAddress(ClrHeap heap, IDataReader reader, ClrEl
             throw new Exception("Unexpected element type.");
         }
 
-        internal static string GetStringContents(ClrType stringType, IDataReader reader, ulong strAddr)
+        internal static string GetStringContents(ClrType stringType, IDataReader reader, ulong strAddr, int maxLen)
         {
             if (strAddr == 0)
                 return null;
@@ -165,6 +165,7 @@ internal static string GetStringContents(ClrType stringType, IDataReader reader,
             int length = _stringLength.Read<int>(strAddr, interior: false);
             ulong data = _firstChar.GetAddress(strAddr);
 
+            length = Math.Min(length, maxLen);
             return ReadString(reader, data, length);
         }