Array fields should not be read only

Article
08/20/2007

TypeName	ArrayFieldsShouldNotBeReadOnly
CheckId	CA2105
Category	Microsoft.Security
Breaking Change	Breaking

Cause

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

Rule Description

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.

How to Fix Violations

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.

When to Exclude Warnings

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.

Example

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

Share via