BUG: refs #981. Fix security vulnerability in web API upload method

Author: zachmullen

core/AppController.php

@@ -118,7 +118,7 @@ public function preDispatch()
                 }
               else
                 {
-                $auth = $this->User->legacyAuthenticate($userDao, '', '', $tmp[1]);
+                $auth = $userModel->legacyAuthenticate($userDao, '', '', $tmp[1]);
                 }
               // if authenticated, set the session user to be this user
               if($auth)

modules/api/controllers/components/ApiComponent.php

@@ -344,40 +344,45 @@ function uploadGeneratetoken($args)
       $existingBitstream = $bitstreamModel->getByChecksum($args['checksum']);
       if($existingBitstream)
         {
-        $revision = $itemModel->getLastRevision($item);
-
-        if($revision == false)
+        // User must have read access to the existing bitstream if they are circumventing the upload.
+        // Otherwise an attacker could spoof the checksum and read a private bitstream with a known checksum.
+        if($itemModel->policyCheck($existingBitstream->getItemrevision()->getItem(), $userDao, MIDAS_POLICY_READ))
           {
-          // Create new revision if none exists yet
-          Zend_Loader::loadClass('ItemRevisionDao', BASE_PATH.'/core/models/dao');
-          $revision = new ItemRevisionDao();
-          $revision->setChanges('Initial revision');
-          $revision->setUser_id($userDao->getKey());
-          $revision->setDate(date('c'));
-          $revision->setLicenseId(null);
-          $itemModel->addRevision($item, $revision);
-          }
+          $revision = $itemModel->getLastRevision($item);
 
-        $siblings = $revision->getBitstreams();
-        foreach($siblings as $sibling)
-          {
-          if($sibling->getName() == $args['filename'])
+          if($revision == false)
+            {
+            // Create new revision if none exists yet
+            Zend_Loader::loadClass('ItemRevisionDao', BASE_PATH.'/core/models/dao');
+            $revision = new ItemRevisionDao();
+            $revision->setChanges('Initial revision');
+            $revision->setUser_id($userDao->getKey());
+            $revision->setDate(date('c'));
+            $revision->setLicenseId(null);
+            $itemModel->addRevision($item, $revision);
+            }
+
+          $siblings = $revision->getBitstreams();
+          foreach($siblings as $sibling)
             {
-            // already have a file with this name. don't add new record.
-            return array('token' => '');
+            if($sibling->getName() == $args['filename'])
+              {
+              // already have a file with this name. don't add new record.
+              return array('token' => '');
+              }
             }
+          Zend_Loader::loadClass('BitstreamDao', BASE_PATH.'/core/models/dao');
+          $bitstream = new BitstreamDao();
+          $bitstream->setChecksum($args['checksum']);
+          $bitstream->setName($args['filename']);
+          $bitstream->setSizebytes($existingBitstream->getSizebytes());
+          $bitstream->setPath($existingBitstream->getPath());
+          $bitstream->setAssetstoreId($existingBitstream->getAssetstoreId());
+          $bitstream->setMimetype($existingBitstream->getMimetype());
+          $revisionModel = MidasLoader::loadModel('ItemRevision');
+          $revisionModel->addBitstream($revision, $bitstream);
+          return array('token' => '');
           }
-        Zend_Loader::loadClass('BitstreamDao', BASE_PATH.'/core/models/dao');
-        $bitstream = new BitstreamDao();
-        $bitstream->setChecksum($args['checksum']);
-        $bitstream->setName($args['filename']);
-        $bitstream->setSizebytes($existingBitstream->getSizebytes());
-        $bitstream->setPath($existingBitstream->getPath());
-        $bitstream->setAssetstoreId($existingBitstream->getAssetstoreId());
-        $bitstream->setMimetype($existingBitstream->getMimetype());
-        $revisionModel = MidasLoader::loadModel('ItemRevision');
-        $revisionModel->addBitstream($revision, $bitstream);
-        return array('token' => '');
         }
       }
     //we don't already have this content, so create the token

modules/api/tests/controllers/ApiCallItemMethodsTest.php

@@ -226,6 +226,8 @@ public function testBitstreamUpload()
      *
      *22 generatetoken passing in folderid and a checksum of an existing bitstream,
      *   and passing Public explicitly
+     *25 generatetoken passing in folderid and checksum of an existing bitstream, but the user
+     *   has no read access to the already existing item, so exception is thrown
      *
      */
 
@@ -415,6 +417,28 @@ public function testBitstreamUpload()
     // delete the newly created item
     $this->Item->delete($itemDao);
 
+    // 25
+    // Just like #22, but we want to make sure the clone existing bitstream behavior
+    // doesn't happen when the user doesn't have read access to the item with the given checksum.
+    // Otherwise for an attacker, having the checksum of a bitstream would allow them to exploit
+    // upload.generatetoken to gain read access to that bitstream, even if they didn't have
+    // read access on it.
+    $existingItem = $this->Item->load(1001); // this should be the item with the existing bitstream in it
+    $policy = $this->Itempolicyuser->getPolicy($usersFile[0], $existingItem);
+    $this->Itempolicyuser->delete($policy); // delete permission for the user on the item
+    $this->resetAll();
+    $this->params['token'] = $this->_loginAsNormalUser();
+    $this->params['method'] = 'midas.upload.generatetoken';
+    $this->params['filename'] = 'some_new_file.txt';
+    $this->params['checksum'] = $md5;
+    $this->params['folderid'] = '1000';
+    $this->params['itemprivacy'] = 'Public';
+    $resp = $this->_callJsonApi();
+    $this->_assertStatusOk($resp);
+    $token = $resp->data->token;
+    $this->assertNotEquals($token, '', 'User needs read access to existing bitstream to clone it');
+    $this->Itempolicyuser->createPolicy($usersFile[0], $existingItem, $policy->getPolicy()); //restore policy state
+
     // 7
     // generate upload token
     // when we generate an upload token for a newly created item with a

modules/api/tests/controllers/ApiCallMethodsTest.php

@@ -27,7 +27,7 @@ public function setUp()
     $this->setupDatabase(array('default')); //core dataset
     $this->setupDatabase(array('default'), 'api'); // module dataset
     $this->enabledModules = array('api');
-    $this->_models = array('User', 'Folder', 'Item', 'ItemRevision', 'Assetstore', 'Bitstream');
+    $this->_models = array('User', 'Folder', 'Item', 'ItemRevision', 'Assetstore', 'Bitstream', 'Itempolicyuser');
     $this->_daos = array();
 
     parent::setUp();