Page History

...

Callback methods are generally invoked from the system with full permissions. It seems reasonable to expect that malicious code needs to be on the stack in order to perform an operation, but that is not the case. Malicious code may set up objects that bridge the callback to a security checked operation. For instance, a file chooser dialog box that can manipulate the filesystem from user actions, may have events posted from malicious code. Alternatively, malicious code can disguise a file chooser as something benign while redirecting user events.

This rule is an instance of 18. SEC51-JG. Minimize privileged code.

Noncompliant Code Example

This noncompliant code example uses a CallBackImpl a UserLookupCallBack class that implements the the CallBack interface . The securityCritical() method accepts a privileged action that is used by its implementation as an argument to a doPrivileged blockto look up a user's name given their ID. This lookup code assumes that this information lives in the /etc/passwd file, which requires elevated privileges to open. Consequently, the Client class invokes all callbacks with elevated privileges (within a doPrivileged block).

public interface CallBack {
  void securityCriticalcallMethod(PrivilegedAction<String> action);
}
 
class CallBackImplUserLookupCallBack implements CallBack {
  private int uid;
  public void securityCriticalUserLookupCallBack(PrivilegedAction<String>int actionuid) {
    AccessController.doPrivileged(action);        this.uid = uid;
  }
}
 
class Client {
  private String name;
  public voidString registergetName(CallBack callback) {
    callback.securityCritical(new MaliciousUserLookupAction(7))return name;
  }

  public static void maincallMethod(String[] args) {
    Client clienttry (InputStream fis = new Client();
    CallBack callBack = new CallBackImpl();
    client.register(callBack);
  }
} 

The class Client allows registering the callback so that an untrusted caller can specify the user id to look-up. An object of type UserLookupAction  is expected by the callback, however, an attacker may extend that class and replace it with a malicious implementation in the form of a MaliciousUserAction instance.

public class UserLookupAction implements PrivilegedAction<String> {
  private int userid;
 
  public UserLookupAction(int useridFileInputStream("/etc/passwd")) {
      // Look up uid & assign to name
    } catch (IOException x) {
      name = null;
    }
  }
}

class Client {
  CallBack callback;

  public void registerCallBack(CallBack callback) {
    this.useridcallback = useridcallback;
  }
 
  public Stringvoid rundoSomething() {
    String name = null;
AccessController.doPrivileged(new PrivilegedAction<Void>() {
     try (InputStream fis =public newVoid FileInputStream("/etc/passwd")run() {
      // Look up userid & assign to name
 callback.callMethod();
       } catch (IOException x)return {null;
      name = null;}
    }
    return name});
  }
}
 
class MaliciousUserLookupAction extends UserLookupAction
  public static void main(String[] args) {
    int public MaliciousUserLookupAction(int userid) {
    super(userid);
  }

  public String run() {uid = Integer.parseInt( args[0]);

    Client client = new Client();
    UserLookupCallBack callBack = new UserLookupCallBack(uid);

    client.registerCallBack(callBack);
    // ...
    client.doSomething(); // looks up user name
    System.out.println("Executing untrusted code");
    return null"User " + uid + " is named " + callBack.getName());
  }
}

...

Whie this code works as expected, an attacker can use it to run malicious code with elevated privileges using code like the following:

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

// ...
Client client = new Client();
client.registerCallBack(new MaliciousCallBack());
client.doSomething(); // performs malicious code

Compliant Solution 

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

By convention, instances of PrivilegedAction and PrivilegedExceptionAction may be made available to untrusted code, but doPrivileged must not be invoked with caller-provided actions.

This compliant solution amends the CallBack interface and instead of accepting the PrivilegedAction objects, the securityCritical() methods accepts the user id to be searched for. The code contained within the UserLookupAction class is moved to the securityCritical() methodmoves the invocation of doPrivileged() out of the Client 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.

class UserLookupCallBackpublic interface CallBack {
  void securityCritical(int uid);
}
 
class CallBackImpl implements CallBack {
  public void securityCriticalcallMethod(int uid) {
    AccessController.doPrivileged(new PrivilegedAction<String>PrivilegedAction<Void>() {
        public StringVoid run() {
        String name;
        try (InputStream fis = new FileInputStream("/etc/passwd")) {
            // Look up userid & assign to UserLookupCallBack.this.name
          } catch (IOException x) {
            UserLookupCallBack.this.name = null;
          }
          return namenull;
        }
      });
  }

  // ... rest of 
UserLookupCallBack  }unchanged
}
  
class Client {
  public void registerdoSomething(CallBack callback) {
    callback.securityCriticalcallMethod(7);
  }

  public static void main(String[] args) {
    Client client = new Client();
    CallBack callBack = new CallBackImpl();
    client.register(callBack);
  }
} 

...

// ...rest of Client unchanged
}

Applicability

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

...