Was this page helpful?
Your feedback about this content is important. Let us know what you think.
Additional feedback?
1500 characters remaining
Array fields should not be read only

Array fields should not be read only

TypeName

ArrayFieldsShouldNotBeReadOnly

CheckId

CA2105

Category

Microsoft.Security

Breaking Change

Breaking

A public or protected field that holds an array is declared read-only.

When you apply the readonly (ReadOnly in Visual Basic) modifier to a field that contains an array, the field cannot be changed to refer to a different array; however, the elements of the array stored in a read-only field can be changed. Code that makes decisions or performs operations based on the elements of a publicly accessible read-only array might contain an exploitable security vulnerability.

Note that having a public field also violates the design rule Do not declare visible instance fields.

To fix the security vulnerability identified by this rule, do not rely on the contents of a publicly accessible read-only array. It is strongly recommended that you do one of the following procedures:

  • Replace the array with a strongly typed collection that cannot be changed. For additional information, see System.Collections.ReadOnlyCollectionBase

  • Replace the public field with a method that returns a clone of a private array. Because your code does not rely on the clone, there is no danger if the elements are modified.

If you chose the second approach, do not replace the field with a property; properties that return arrays have a negative effect on performance. For more information, see Properties should not return arrays.

Excluding a warning from this rule is strongly discouraged. There are virtually no scenarios where the contents of a read-only field are unimportant. If this is the case with your scenario, remove the readonly modifier instead of excluding the message.

This example demonstrates the dangers of violating this rule. The first part shows an example library with a type, MyClassWithReadOnlyArrayField, containing two fields (grades and privateGrades) that are not secure. The field grades is public, and therefore vulnerable to any caller. The field privateGrades is private, but is still vulnerable because it is returned to callers by the GetPrivateGrades method. The securePrivateGrades field is exposed in a safe manner by the GetSecurePrivateGrades method. It is declared as private to follow good design practices. The second part shows code that changes values stored in the grades and privateGrades members.

The example class library appears below.

using System;

namespace SecurityRulesLibrary
{
   public class MyClassWithReadOnlyArrayField
   {
      public readonly int[] grades = {90, 90, 90};
      private readonly int[] privateGrades = {90, 90, 90};
      private readonly int[] securePrivateGrades = {90, 90, 90};

      // Making the array private does not protect it because it is passed to others.
      public int[] GetPrivateGrades()
      {
         return privateGrades;
      }
      //This method secures the array by cloning it.
      public int[] GetSecurePrivateGrades()
      {
            return (int[])securePrivateGrades.Clone();
      }

      public override string ToString() 
      {
         return String.Format("Grades: {0}, {1}, {2} Private Grades: {3}, {4}, {5}  Secure Grades, {6}, {7}, {8}", 
            grades[0], grades[1], grades[2], privateGrades[0], privateGrades[1], privateGrades[2], securePrivateGrades[0], securePrivateGrades[1], securePrivateGrades[2]);
      }     
   }
}
   

The following code uses the example class library to illustrate read-only array security issues.

using System;
using SecurityRulesLibrary;

namespace TestSecRulesLibrary
{
   public class TestArrayReadOnlyRule
   {
      [STAThread]
      public static void Main() 
      {
         MyClassWithReadOnlyArrayField dataHolder = 
            new MyClassWithReadOnlyArrayField();

         // Get references to the library's readonly arrays.
         int[] theGrades = dataHolder.grades;
         int[] thePrivateGrades = dataHolder.GetPrivateGrades();
         int[] theSecureGrades = dataHolder.GetSecurePrivateGrades();

         Console.WriteLine(
            "Before tampering: {0}", dataHolder.ToString());

         // Overwrite the contents of the "readonly" array. 
         theGrades[1]= 555;
         thePrivateGrades[1]= 555;
         theSecureGrades[1]= 555;
         Console.WriteLine(
            "After tampering: {0}",dataHolder.ToString());
      }
   }
}

The output from this example is:

Output

Before tampering: Grades: 90, 90, 90 Private Grades: 90, 90, 90  Secure Grades, 90, 90, 90
After tampering: Grades: 90, 555, 90 Private Grades: 90, 555, 90  Secure Grades, 90, 90, 90

Community Additions

ADD
Show:
© 2015 Microsoft