[Mono-bugs] [Bug 47707][Cri] New - Missing bounds checking in Array.IList.this[]

[bugzilla-daemon@bugzilla.ximian.com]

Please do not reply to this email- if you want to comment on the bug, go to the
URL shown below and enter your comments there.

Changed by lb@lb.ods.org.

http://bugzilla.ximian.com/show_bug.cgi?id=47707

--- shadow/47707	2003-08-18 13:04:02.000000000 -0400
+++ shadow/47707.tmp.31804	2003-08-18 13:04:02.000000000 -0400
@@ -0,0 +1,107 @@
+Bug#: 47707
+Product: Mono/Class Libraries
+Version: unspecified
+OS: All
+OS Details: 
+Status: NEW   
+Resolution: 
+Severity: 
+Priority: Critical
+Component: CORLIB
+AssignedTo: mono-bugs@ximian.com                            
+ReportedBy: lb@lb.ods.org               
+QAContact: mono-bugs@ximian.com
+TargetMilestone: ---
+URL: 
+Cc: 
+Summary: Missing bounds checking in Array.IList.this[]
+
+Description of Problem:
+Array.GetValueImpl and Array.SetValueImpl are implemented in unmanaged code
+without bounds checking.
+
+This is sensible since it is faster and allows managed code to freely
+handle out-of-bounds cases according the specification; however it means
+that managed code MUST ensure that the parameter it passes to the two
+functions is within bounds.
+
+
+Unfortunately, there is code that doesn't bother to do so:
+
+// IList interface
+object IList.this [int index] {
+	get {
+		return GetValueImpl (index);
+	} 
+	set {
+		SetValueImpl (value, index);
+	}
+}
+[This seems the only such code, but I don't make any guarantee]
+
+
+Test cases:
+	public volatile object volatile_obj = null;
+
+#if NET_1_1
+	[Test]
+	[ExpectedException (typeof (ArgumentOutOfRangeException))]
+	public void TestIList_Item_GetLowerBound()
+	{
+		object[] a = new object[16];
+		IList l = (IList)a;
+		volatile_obj = l[-1];
+	}
+
+	[Test]
+	[ExpectedException (typeof (ArgumentOutOfRangeException))]
+	public void TestIList_Item_GetUpperBound()
+	{
+		object[] a = new object[16];
+		IList l = (IList)a;
+		volatile_obj = l[16];
+	}
+
+	[Test]
+	[ExpectedException (typeof (ArgumentOutOfRangeException))]
+	public void TestIList_Item_SetLowerBound()
+	{
+		object[] a = new object[16];
+		IList l = (IList)a;
+		l[-1] = volatile_obj;
+	}
+
+	[Test]
+	[ExpectedException (typeof (ArgumentOutOfRangeException))]
+	public void TestIList_Item_SetUpperBound()
+	{
+		object[] a = new object[16];
+		IList l = (IList)a;
+		l[16] = volatile_obj;
+	}
+#endif
+
+Steps to reproduce the problem:
+1. Run the test case or equivalent code
+
+Actual Results:
+If an access violation happens, the following:
+ 
+Unhandled Exception: System.NullReferenceException: A null value was found
+where an object instance was required
+in (unmanaged) /usr/lib/libmono.so.0(mono_value_box+0xde) [0x4008e7aa]
+in (unmanaged) /usr/lib/libmono.so.0 [0x400905ac]
+in <0x00015> 00 System.Array:System.Collections.IList.get_Item (int)
+in <0x00057> 00 .App:Main ()
+
+Otherwise, it reads/writes to an arbitrary attacker-selectable memory
+address (on 64-bit systems some addresses may not be reachable), which is a
+complete security bypass and a specification violation.
+
+
+Patch:
+Should be attached.
+
+The idea of the unchecked unsigned casts is to do both lower and upper
+bounds checking in a single operation since negative values get mapped to
+0x80000000-0xffffffff.