Defuse_-_ZeroBin.txt

-----------------------------------------------------------------------
                       Security Audit of ZeroBin
                             Taylor Hornby
                           February 02, 2014
-----------------------------------------------------------------------

1. Introduction

   ZeroBin's website [ZBW] describes ZeroBin as "a minimalist,
   opensource online pastebin/discussion board where the server has zero
   knowledge of hosted data."

   This report documents the results of a 4-hour security audit of
   ZeroBin. It was performed for free as a service to the free and open
   source software community.

   The audit was performed on a current clone of ZeroBin's Git
   repository [GR]. The commit hash is:

     4f8750bbddcb137213529875e45e3ace3be9a769

   Note that this code is newer than the code available for download
   from the ZeroBin website [ZBW] (version 0.18), which has known
   vulnerabilities [XSS].

1.1 Audit Scope

   This audit focused on the PHP side of ZeroBin. The JavaScript side
   was only audited briefly, so most issues could not be verified, and
   were added to Section 4, Future Work, below. The audit did NOT check
   for XSS vulnerabilities in zerobin.js, and did NOT check for correct
   usage of the SJCL cryptography library.

   The audit did NOT cover the following files:

     - lib/rain.tpl.class.php
     - js/base64.js
     - js/highlight.pack.js
     - js/jquery.js
     - js/rawdeflate.js
     - js/rawinflate.js
     - js/sjcl.js

2. Issues

2.1. Salt and HMAC Key Generated with mt_rand()

   Exploitability: Medium
   Security Impact: Low

   In serversalt.php, generateRandomSalt() generates a salt using the
   mt_rand() pseudo-random number generator. Because mt_rand() is not
   a cryptographic random number generator, this function will return
   easily-guessed salts with a much higher probability of collision.

   The salts generated by this function are relied on for security. For
   example, it is used as an HMAC key when checking delete tokens in
   processPasteDelete(). It should be replaced with mcrypt_create_iv()
   or openssl_random_pseudo_bytes().

   Another unused mt_rand() salt generator appears in
   vizhash_gd_zero.php. This should be removed.

   This issue has already been reported in [GH68], but has not been
   fixed (the issue is still open).

2.2. Fixed Server Salt

   Exploitability: Low
   Security Impact: Low

   The security of the VizHash hashes of IP addresses (used to generate
   comment avatars) and delete tokens both rely on a fixed "salt" which
   is generated once per deployment, saved in data/salt.php.

   As noted in Issue 2.1, this salt is not generated with a secure
   random number, but keeping it fixed has several other disadvantages:

     - If the salt is compromised and published, anyone can reverse
       a VizHash into the corresponding IP address, even for expired
       posts (that they saved). This would not be possible if a random
       comment salt was generated for each post, then destroyed along
       with the post when it expires. This would, however, give users
       different avatars on different posts (which may be desired).

     - If the salt is compromised and published, anyone can find the
       delete link for any post, since they can compute the HMAC. This
       does not need to be possible. A random delete token could be
       generated for each post, and its hash stored along with the post.
       Then, the post could only be deleted if the user can provide the
       preimage of the hash.

     - Reusing the same key for two purposes is generally a bad idea.

2.3. Traffic Limiter Race Conditions

   To rate limit requests, ZeroBin keeps a history of hit times in
   data/traffic_limiter.php, which is generated and re-generated in
   traffic_limiter_canPass(). Because requests can be made
   asynchronously, the file may become corrupted.

   This might allow remote code execution, if the attacker is clever
   enough to corrupt the file in just the right way, since the
   traffic_limiter.php file is require()'d.

   This issue has already been reported, and a patch has been provided,
   in [GH61], but has not been fixed (the issue is still open).

2.4. VizHash IP Address Online Guessing

   Exploitability: Very Low
   Security Impact: Low

   The VizHash system is used to give users who comment an avatar
   derived from their IP address. If an attacker can create connections
   to the ZeroBin server from arbitrary source IPs (e.g. if they are in
   the same LAN as the ZeroBin server, or man-in-the-middling its
   traffic), they can perform an online guessing attack on the VizHash
   they are interested in.

2.5. Relies on .htaccess files, which may not be enabled.

   Exploitability: N/A
   Security Impact: Very Low

   ZeroBin relies on .htaccess files being enabled to prevent access to
   the 'data' directory. This directory contains the ciphertexts, salt,
   and rate limiter array. If .htaccess is disabled, of if ZeroBin is
   installed on a non-Apache web server, then an attacker may be able to
   access these files. 
   
   The salt and rate limiter array are safe since they are in .php
   files. However, the attacker would be able to access the ciphertext.
   This is not a security issue because the attacker already has access
   to the ciphertext.

   If directory traversal is enabled, the attacker can see all of the
   post ids, too.

2.6. The robots.txt does not work in a subdomain.

   Exploitability: N/A
   Security Impact: Low

   ZeroBin uses a robots.txt file to prevent search engines from
   indexing the posts. If you install ZeroBin into a subdirectory, this
   does not work. A note about this should be added to the README or
   other documentation, so that the user knows to install the robots.txt
   file in the right place.

2.7 HMAC Not Compared in Constant Time

   Exploitability: Medium
   Security Impact: Low

   ZeroBin uses an HMAC to generate and check delete tokens. The HMAC is
   compared against the correct one with PHP's "!=" operator. A timing
   attack can exploit this error to compute the HMAC of arbitrary data
   with the server's salt.

   ZeroBin should check if the strings are equal in constant time:

     function slow_equals($a, $b)
     {
         $diff = strlen($a) ^ strlen($b);
         for($i = 0; $i < strlen($a) && $i < strlen($b); $i++)
         {
             $diff |= ord($a[$i]) ^ ord($b[$i]);
         }
         return $diff === 0;
     }

2.8. Arbitrary File Unlink

   Exploitability: Medium
   Security Impact: High

   An attacker can delete arbitrary files on the server by exploiting
   a vulnerability in processPasteDelete(). Here is a condensed version
   of the code:

   function processPasteDelete($pasteid,$deletetoken)
   {
       if (preg_match('/\A[a-f\d]{16}\z/',$pasteid))  
       {
           $filename = dataid2path($pasteid).$pasteid;
           if (!is_file($filename)) 
           {
               return ... error message ...
           }
       }
   
       if ($deletetoken != hash_hmac('sha1', $pasteid , getServerSalt())) 
       {
           return ... error message ...
       }
   
       deletePaste($pasteid);
       return array('','','Paste was properly deleted.');
   }

   The $pasteid variable comes directly from $_GET['pasteid'], which the
   attacker can control. The $deletetoken variable comes from
   $_GET['deletetoken']. If $deletetoken matches the HMAC, $pasteid is
   passed to deletePaste(), which runs:
   
     unlink(dataid2path($pasteid).$pasteid);

    Thus, if an attacker can compute the HMAC, they can delete arbitrary
    files on the server. The HMAC can be computed if the salt is known.
    Forging the HMAC without already knowing the salt is easy because of
    Issue 2.7 and Issue 2.1.

2.9. HMAC Uses SHA1 instead of SHA256

   Exploitability: Very Low
   Security Impact: Low

   ZeroBin uses a SHA1 HMAC to derive the delete token. SHA1 should be
   replaced with a better hash function, like SHA256.

2.10. No Cross-Site Request Forgery Protection

   Exploitability: Medium
   Security Impact: Medium

   ZeroBin does not attempt to prevent Cross-Site Request Forgery (CSRF)
   attacks. A malicious website can make a user's browser delete ZeroBin
   posts (if the delete token is known), create posts, and post comments
   to existing posts.

   A CSRF attack to post comments can be a problem when a malicious
   website wants to find out the ZeroBin VizHash of its visitors, to see
   what they have been commenting. The attack would work as follows:

     1. Alice suspects Bob posted a rude ZeroBin comment.
     2. Alice generates a web page that causes Bob's browser to comment
        "I AM BOB!" on a ZeroBin post.
     3. Alice emails a link to the malicious page to Bob.
     4. Bob clicks the link.
     5. Alice checks the post, sees that the "I AM BOB!" comment has the
        same VizHash as the rude comment. Alice now knows it was Bob
        that made the rude comment.

3. Coding Practices

   The ZeroBin code could be made more secure, and easier to audit, by
   following some good coding practices. These are documented in the
   next sections.

3.1. Always Assume Malice

   When a string is used in a way that might affect security, it should
   always be assumed to be malicious, even if it is just a constant
   string. The ZeroBin code does not do this, for example:

     - In the post creation code, it assumes $dataid is safe, which it
       is, since it it is a hex string from an MD5 hash, but it would be
       better if the code assumed it was malicious and checked/escaped
       it anyway.

     - Several strings are not escaped in tpl/page.html. $VERSION is not
       escaped in the header, and $STATUS is not escaped in the body.
       Even though these are static strings, they should be escaped
       anyway, since someone might change the error messages to reflect
       user input in the future. See Section 4.6.

4. Future Work

4.1. Secure Code Delivery

   ZeroBin relies on the JavaScript code being delivered securely.
   A malicious server or man-in-the-middle can modify the code to leak
   the key. This is already well known and is being addressed in [GH17].

4.2. urls2links XSS

   In zerobin.js, urls2links() converts a URL text into a clickable
   link. The replacement is done purely by regular expression, and no
   escaping is done. This may be a source of XSS vulnerabilities. I did
   not have enough time to understand what the regular expression is
   doing, so I'm not sure if this is exploitable or not.

   There are other bits of code that look like they might be XSS
   vulnerabilities (e.g. line 233 in zerobin.js where divComment is
   set). I did not have enough time to check these in detail.

4.3. Low Entropy in Browsers Without CSPRNGs

   If the web browser does not have window.crypto.getRandomValues(), the
   key is derived from mouse movements and stuff. That may not be good
   enough. Perhaps in these cases the server could help the client by
   supplying some of its own entropy from mcrypt_create_iv() or
   openssl_random_pseudo_bytes().

4.4. Plaintext is Compressed Before Encryption

   Posts are compressed before they are encrypted. This makes the
   ciphertext length depend on the plaintext content. This may create
   vulnerabilities in specific use cases where an attacker can choose
   variations of the same post to be encrypted.  This would be similar
   to the CRIME and BREACH attacks, except happening at the application
   layer.

4.5. Is SJCL used correctly?

   This audit did not check if zerobin.js uses the SJCL cryptography
   library correctly.

4.6. Possible XSS in tpl/page.html

   JSON encoded data is inserted into the HTML page unescaped in
   tpl/page.html. This is because $STATUS is set to the return value of
   processPasteFetch(), which returns JSON encoded data based on the
   user's input. I did not check if this is exploitable.

5. Conclusion

   Several issues were found. The most critical being the arbitrary file
   unlink vulnerability, described in Section 2.8, and the use of
   mt_rand() to generate HMAC keys, described in Section 2.1.

   This audit did not cover the JavaScript cryptography, nor did it
   cover XSS vulnerabilities in the JavaScript code. More auditing time
   is recommended.

6. References

   [GH17] https://github.com/sebsauvage/ZeroBin/issues/17
   [GH68] https://github.com/sebsauvage/ZeroBin/issues/68
   [GH61] https://github.com/sebsauvage/ZeroBin/pull/61
   [GR]   https://github.com/sebsauvage/ZeroBin
   [ZBW]  http://sebsauvage.net/wiki/doku.php?id=php:zerobin
   [XSS]  https://github.com/sebsauvage/ZeroBin/commit/4f8750bbddcb137213529875e45e3ace3be9a769