From 93517c8e42fb1ddfdcc304839329c23d1f09685e Mon Sep 17 00:00:00 2001
From: "Ray J. Greenwell" <ray@threerings.net>
Date: Wed, 18 Jul 2012 17:32:18 -0700
Subject: [PATCH] Disable "manager calls" by default. **Breaking change**

This closes a giant security hole, but may break existing code.

PlaceObject.ManagerCaller is a poor-man's InvocationService, allowing
one to easily make a call to a method on the PlaceManager. The problem
is that this is turned on by default and any matching method is run.

This change disables it by default, but you may re-enable it by
overriding PlaceManager.handleManagerCalls() and returning true.

I also went ahead and made the ManagerCaller.invoke() method deprecated
so that you'll see a nice warning during compile time. You should
really consider just using a standard InvocationService to communicate with
the server. That way you effectively create a whitelist of client-callable
methods and there will be no surprises.
---
 .../com/threerings/crowd/data/PlaceObject.java     |  8 ++++++++
 .../com/threerings/crowd/server/PlaceManager.java  | 14 ++++++++++++++
 2 files changed, 22 insertions(+)

diff --git a/core/src/main/java/com/threerings/crowd/data/PlaceObject.java b/core/src/main/java/com/threerings/crowd/data/PlaceObject.java
index bac39c310..bdba8e978 100644
--- a/core/src/main/java/com/threerings/crowd/data/PlaceObject.java
+++ b/core/src/main/java/com/threerings/crowd/data/PlaceObject.java
@@ -64,9 +64,17 @@ public class PlaceObject extends DObject
      *
      * and to route events through the right distributed object manager if we are running in
      * standalone/single-player mode where both client and server are running in the same VM.
+     *
+     * <em>Note:</em> For this to work, your manager needs to override
+     *    <code>PlaceManager.handleManagerCalls()</code> and return true. But maybe you should
+     *    consider using an InvocationService instead.
      */
     public class ManagerCaller
     {
+        /**
+         * @deprecated this is pretty darn unsafe. Why don't you just set up a Service?
+         */
+        @Deprecated
         public void invoke (String method, Object ... args) {
             _omgr.postEvent(new ServerMessageEvent(_oid, method, args));
         }
diff --git a/core/src/main/java/com/threerings/crowd/server/PlaceManager.java b/core/src/main/java/com/threerings/crowd/server/PlaceManager.java
index 8d718f6e1..cac595f2f 100644
--- a/core/src/main/java/com/threerings/crowd/server/PlaceManager.java
+++ b/core/src/main/java/com/threerings/crowd/server/PlaceManager.java
@@ -385,6 +385,11 @@ public class PlaceManager
             int srcoid = event.getSourceOid();
             DObject source = (srcoid <= 0) ? null : _omgr.getObject(srcoid);
             Object[] args = event.getArgs(), nargs;
+            if (!handleManagerCalls()) {
+                log.warning("Client tried to invoke a manager call!",
+                    "source", source, "args", args);
+                return;
+            }
             if (args == null) {
                 nargs = new Object[] { source };
             } else {
@@ -451,6 +456,15 @@ public class PlaceManager
         return buf.toString();
     }
 
+    /**
+     * Do we want to allow client code to invoke methods by name?
+     * By default, we do not.
+     */
+    protected boolean handleManagerCalls ()
+    {
+        return false;
+    }
+
     /**
      * Derived classes will generally override this method to create a custom {@link PlaceObject}
      * derivation that contains extra information.