Export (0) Print
Expand All
Expand Minimize
This topic has not yet been rated - Rate this topic

CA2105: 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 that are stored in a read-only field can be changed. Code that makes decisions or performs operations that are based on the elements of a read-only array that can be publicly accessed might contain an exploitable security vulnerability.

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

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

  • Replace the array with a strongly typed collection that cannot be changed. For more 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 adversely affect performance. For more information, see CA1819: Properties should not return arrays.

Exclusion of a warning from this rule is strongly discouraged. Almost no scenarios occur 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 that has a type, MyClassWithReadOnlyArrayField, that contains 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 in the following example.

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:

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
Show:
© 2014 Microsoft. All rights reserved.