From 05c1776ada78438d73ac5e8c2e4d154aee38a268 Mon Sep 17 00:00:00 2001
From: El RIDO <elrido@gmx.net>
Date: Sun, 27 May 2018 14:36:30 +0200
Subject: [PATCH] ensure ALL read errors are only exposed in the JSON API to
 avoid information leakage (i.e. beviour for deleted vs expired pastes),
 updated test cases & removed duplicate test

---
 lib/PrivateBin.php     | 31 ++++++++++++-------------------
 tst/PrivateBinTest.php | 37 ++++++++++---------------------------
 2 files changed, 22 insertions(+), 46 deletions(-)

diff --git a/lib/PrivateBin.php b/lib/PrivateBin.php
index 1b16434..ef35301 100644
--- a/lib/PrivateBin.php
+++ b/lib/PrivateBin.php
@@ -356,38 +356,31 @@ class PrivateBin
     }
 
     /**
-     * Read an existing paste or comment
+     * Read an existing paste or comment, only allowed via a JSON API call
      *
      * @access private
      * @param  string $dataid
      */
     private function _read($dataid)
     {
+        if (!$this->_request->isJsonApiCall()) {
+            return;
+        }
+
         try {
             $paste = $this->_model->getPaste($dataid);
             if ($paste->exists()) {
-                // reading paste is only possible via JSON call
-                if ($this->_request->isJsonApiCall()) {
-                    $data              = $paste->get();
-                    $this->_doesExpire = property_exists($data, 'meta') && property_exists($data->meta, 'expire_date');
-                    if (property_exists($data->meta, 'salt')) {
-                        unset($data->meta->salt);
-                    }
-                    $this->_data = json_encode($data);
+                $data              = $paste->get();
+                $this->_doesExpire = property_exists($data, 'meta') && property_exists($data->meta, 'expire_date');
+                if (property_exists($data->meta, 'salt')) {
+                    unset($data->meta->salt);
                 }
+                $this->_return_message(0, $dataid, (array) $data);
             } else {
-                $this->_error = self::GENERIC_ERROR;
+                $this->_return_message(1, self::GENERIC_ERROR);
             }
         } catch (Exception $e) {
-            $this->_error = $e->getMessage();
-        }
-
-        if ($this->_request->isJsonApiCall()) {
-            if (strlen($this->_error)) {
-                $this->_return_message(1, $this->_error);
-            } else {
-                $this->_return_message(0, $dataid, json_decode($this->_data, true));
-            }
+            $this->_return_message(1, $e->getMessage());
         }
     }
 
diff --git a/tst/PrivateBinTest.php b/tst/PrivateBinTest.php
index e712a0a..7b953b0 100644
--- a/tst/PrivateBinTest.php
+++ b/tst/PrivateBinTest.php
@@ -679,16 +679,15 @@ class PrivateBinTest extends PHPUnit_Framework_TestCase
      */
     public function testReadInvalidId()
     {
-        $_SERVER['QUERY_STRING'] = 'foo';
+        $_SERVER['QUERY_STRING']          = 'foo';
+        $_SERVER['HTTP_X_REQUESTED_WITH'] = 'JSONHttpRequest';
         ob_start();
         new PrivateBin;
         $content = ob_get_contents();
         ob_end_clean();
-        $this->assertRegExp(
-            '#<div[^>]*id="errormessage"[^>]*>.*Invalid paste ID\.#s',
-            $content,
-            'outputs error correctly'
-        );
+        $response = json_decode($content, true);
+        $this->assertEquals(1, $response['status'], 'outputs error status');
+        $this->assertEquals('Invalid paste ID.', $response['message'], 'outputs error message');
     }
 
     /**
@@ -696,16 +695,15 @@ class PrivateBinTest extends PHPUnit_Framework_TestCase
      */
     public function testReadNonexisting()
     {
-        $_SERVER['QUERY_STRING'] = Helper::getPasteId();
+        $_SERVER['QUERY_STRING']          = Helper::getPasteId();
+        $_SERVER['HTTP_X_REQUESTED_WITH'] = 'JSONHttpRequest';
         ob_start();
         new PrivateBin;
         $content = ob_get_contents();
         ob_end_clean();
-        $this->assertRegExp(
-            '#<div[^>]*id="errormessage"[^>]*>.*Paste does not exist, has expired or has been deleted\.#s',
-            $content,
-            'outputs error correctly'
-        );
+        $response = json_decode($content, true);
+        $this->assertEquals(1, $response['status'], 'outputs error status');
+        $this->assertEquals('Paste does not exist, has expired or has been deleted.', $response['message'], 'outputs error message');
     }
 
     /**
@@ -779,21 +777,6 @@ class PrivateBinTest extends PHPUnit_Framework_TestCase
         $this->assertEquals(0, $response['comment_offset'], 'outputs comment_offset correctly');
     }
 
-    /**
-     * @runInSeparateProcess
-     */
-    public function testReadInvalidJson()
-    {
-        $_SERVER['QUERY_STRING']          = Helper::getPasteId();
-        $_SERVER['HTTP_X_REQUESTED_WITH'] = 'JSONHttpRequest';
-        ob_start();
-        new PrivateBin;
-        $content = ob_get_contents();
-        ob_end_clean();
-        $response = json_decode($content, true);
-        $this->assertEquals(1, $response['status'], 'outputs error status');
-    }
-
     /**
      * @runInSeparateProcess
      */