Uploaded image for project: 'Rules Repository'
  1. Rules Repository
  2. RSPEC-2384

Mutable members should not be stored or returned directly

    Details

    • Message:
      (Store|Return) a copy of "xxx" [or use an immutable "Collection"].
    • Default Severity:
      Minor
    • Impact:
      Low
    • Likelihood:
      Low
    • Targeted languages:
      C#, C++
    • Covered Languages:
      Java
    • Remediation Function:
      Constant/Issue
    • Constant Cost:
      5min
    • Analysis Scope:
      Main Sources
    • CERT:
      OBJ05-J., OBJ06-J., OBJ13-J.
    • CWE:
      CWE-375, CWE-374
    • FindBugs:
      EI_EXPOSE_REP,EI_EXPOSE_REP2,EI_EXPOSE_STATIC_REP2, MS_EXPOSE_REP
    • PMD:
      ArrayIsStoredDirectly, MethodReturnsInternalArray

      Description

      Mutable objects are those whose state can be changed. For instance, an array is mutable, but a String is not. Mutable class members should never be returned to a caller or accepted and stored directly. Doing so leaves you vulnerable to unexpected changes in your class state.

      Instead use an unmodifiable Collection (via Collections.unmodifiableCollection, Collections.unmodifiableList, ...) or make a copy of the mutable object, and store or return the copy instead.

      This rule checks that arrays, collections and Dates are not stored or returned directly.

      Noncompliant Code Example

      class A {
        private String [] strings;
      
        public A () {
          strings = new String[]{"first", "second"};
        }
      
        public String [] getStrings() {
          return strings; // Noncompliant
        }
      
        public void setStrings(String [] strings) {
          this.strings = strings;  // Noncompliant
        }
      }
      
      public class B {
      
        private A a = new A();  // At this point a.strings = {"first", "second"};
      
        public void wreakHavoc() {
          a.getStrings()[0] = "yellow";  // a.strings = {"yellow", "second"};
        }
      }
      

      Compliant Solution

      class A {
        private String [] strings;
      
        public A () {
          strings = new String[]{"first", "second"};
        }
      
        public String [] getStrings() {
          return strings.clone();
        }
      
        public void setStrings(String [] strings) {
          this.strings = strings.clone();
        }
      }
      
      public class B {
      
        private A a = new A();  // At this point a.strings = {"first", "second"};
      
        public void wreakHavoc() {
          a.getStrings()[0] = "yellow";  // a.strings = {"first", "second"};
        }
      }
      

      See

      • MITRE, CWE-374 - Passing Mutable Objects to an Untrusted Method
      • MITRE, CWE-375 - Returning a Mutable Object to an Untrusted Caller
      • CERT, OBJ05-J. - Do not return references to private mutable class members
      • CERT, OBJ06-J. - Defensively copy mutable inputs and mutable internal components
      • CERT, OBJ13-J. - Ensure that references to mutable objects are not exposed

        Attachments

          Issue Links

            Activity

              People

              • Assignee:
                Unassigned
                Reporter:
                ann.campbell.2 Ann Campbell
              • Votes:
                0 Vote for this issue
                Watchers:
                3 Start watching this issue

                Dates

                • Created:
                  Updated: