Versions Compared

Key

  • This line was added.
  • This line was removed.
  • Formatting was changed.
Comment: added more info and a CS, pls review

...

This guideline is an instance of 17. Minimize privileged code and is related to SEC01-J. Do not allow tainted variables in privileged blocks.

Noncompliant Code Example

...

Code Block
bgColor#ffcccc
langjava
public interface CallBack {
  void callMethod();
}
 
class UserLookupCallBack implements CallBack {
  private int uid;
  private String name;

  public UserLookupCallBack(int uid) {
    this.uid = uid;
  }

  public String getName() {
    return name;
  }

  public void callMethod() {
    try (InputStream fis = new FileInputStream("/etc/passwd")) {
      // Look up uid & assign to name
    } catch (IOException x) {
      name = null;
    }
  }
}

final class CallBackAction {
  private CallBack callback;

  public CallBackAction(CallBack callback) {
    this.callback = callback;
  }
 
  public void perform() {
    AccessController.doPrivileged(new PrivilegedAction<Void>() {
        public Void run() {
          callback.callMethod();
          return null;
        }
      });
  }
}

...

Code Block
class MaliciousCallBack implements CallBack {
  public void callMethod() {
    // Code here gets executed with elevated privileges
  }
}

// Client code
public static void main(String[] args) {
  CallBack callBack = new MaliciousCallBack();
  CallBackAction action = new CallBackAction(callBack);
  action.perform(); // Executes malicious code
}

Compliant

...

Solution (callback-local doPrivileged block)

According to Oracle's secure coding guidelines [SCG 2010]:

...

This compliant solution moves the invocation of doPrivileged() out of the CallBackAction code and into the callback itself. This code functions the same as before, but an attacker can no longer run malicious callback code with elevated privileges.

Code Block
bgColor#ccccff
langjava
public interface CallBack {
  void callMethod();
}
  
class UserLookupCallBack implements CallBack {
  private int uid;
  private String name;
 
  public UserLookupCallBack(int uid) {
    this.uid = uid;
  }
 
  public String getName() {
    return name;
  }
 
  public void callMethod() {
    AccessController.doPrivileged(new PrivilegedAction<Void>() {
        public Void run() {
          try (InputStream fis = new FileInputStream("/etc/passwd")) {
            // Look up userid & assign to UserLookupCallBack.this.name
          } catch (IOException x) {
            UserLookupCallBack.this.name = null;
          }
          return null;
        }
      });
  }
}
 
final class CallBackAction {
  private CallBack callback;

  public CallBackAction(CallBack callback) {
    this.callback = callback;
  }
 
  public void perform() {
    callback.callMethod();
  }
}

This code functions the same as before, but an attacker can no longer run malicious callback code with elevated privileges. Even though an attacker can pass a malicious callback instance using the constructor of class CallBackAction, the code is not executed with elevated privileges because the malicious instance must contain a doPrivileged block which cannot have the same privileges as trusted code. Additionally, class CallBackAction cannot be subclassed to override the the perform() method as it is declared final.

Compliant Solution (declare callback final)

This compliant solution declares the UserLookupCallBack class final to prevent overriding of callMethod().

Code Block
bgColor#ccccff
langjava
final class UserLookupCallBack implements CallBack {
  // ...    
}
 
// Other code is same

Applicability

Exposing sensitive methods through callbacks can result in misuse of privileges and arbitrary code execution.

...