Alexander Nicolaysen Sørnes : objectManager: Prevent changing variables prior to permission checks in form

[Chris Morgan]

Module: appdb
Branch: master
Commit: b6f1f2219243a740492f4eb2e75a6e8501a3f574
URL:    http://source.winehq.org/git/appdb.git/?a=commit;h=b6f1f2219243a740492f4eb2e...

Author: Alexander Nicolaysen Sørnes <alex(a)thehandofagony.com>
Date:   Wed Oct 24 10:59:22 2007 +0200

objectManager: Prevent changing variables prior to permission checks in form

processing

---

 include/objectManager.php |   16 +++++++++++-----
 1 files changed, 11 insertions(+), 5 deletions(-)

diff --git a/include/objectManager.php b/include/objectManager.php
index 07a8d90..c9ae56f 100644
--- a/include/objectManager.php
+++ b/include/objectManager.php
@@ -519,14 +519,17 @@ class ObjectManager
         $this->checkMethods(array("delete", "canEdit"));
 
         $oObject = $this->getObject();
+        $oOriginalObject = new $this->sClass($this->iId); /* Prevent possible security hole if users change key
+                                                             variables, making the permission checks run on
+                                                             the wrong criteria */
 
-        if(!$oObject->objectGetId())
+        if(!$oOriginalObject->objectGetId())
         {
             addmsg("No id defined", "red");
             return FALSE;
         }
 
-        if(!$oObject->canEdit())
+        if(!$oOriginalObject->canEdit())
         {
             addmsg("You don’t have permission to delete this entry", "red");
             return FALSE;
@@ -923,6 +926,9 @@ class ObjectManager
         $this->iId = $this->getIdFromInput($aClean);
 
         $oObject = new $this->sClass($this->iId);
+        $oOriginalObject = new $this->sClass($this->iId);  /* Prevent possible security hole if users change key
+                                                              variables, making the permission checks run on
+                                                              the wrong criteria */
 
         /* If it isn't implemented, that means there is no default text */
         if(method_exists(new $this->sClass, "getDefaultReply"))
@@ -968,13 +974,13 @@ class ObjectManager
                 // otherwise we should create the entry in the 'else' case
                 if($this->iId)
                 {
-                    if(!$oObject->canEdit())
+                    if(!$oOriginalObject->canEdit())
                         return FALSE;
 
                     if($this->bIsRejected)
                         $oObject->ReQueue();
 
-                    if($this->bIsQueue && !$oObject->mustBeQueued())
+                    if($this->bIsQueue && !$oOriginalObject->mustBeQueued())
                         $oObject->unQueue();
 
                     $oObject->update();
@@ -987,7 +993,7 @@ class ObjectManager
                 break;
 
             case "Reject":
-                if(!$oObject->canEdit())
+                if(!$oOriginalObject->canEdit())
                     return FALSE;
 
                 $oObject->reject();