Merge pull request #130 from mebjas/loggerFix

Change default logger to be based on php's error_log
mebjas · May 23, 2020 · 98b60b0 · 98b60b0
2 parents 563132a + 5e776cf
commit 98b60b0
Show file tree

Hide file tree

Showing 16 changed files with 263 additions and 398 deletions.
diff --git a/libs/README.md b/libs/README.md
@@ -2,8 +2,6 @@ CSRFProtector configuration
 ==========================================
 
  - `CSRFP_TOKEN`: name of the csrf nonce, used for cookie or posting as argument. default: `CSRFP-Token` (if left blank)
- - `logDirectory`: location of the directory at which log files will be saved, either **relative** to the default `config.php` file location or an **absolute** path. This is required for file based logging (default), Not needed, in case you override logging function to implement your logging logic. (View [Overriding logging function](https://github.com/mebjas/CSRF-Protector-PHP/wiki/Overriding-logging-function))
- <br>**Default value:** `../log/`
  - `failedAuthAction`: Action code (integer) for action to be taken in case of failed validation. Has two different values for bot `GET` and `POST`. Different action codes are specified as follows, (<br>**Default:** `0` for both `GET` & `POST`):
     *  `0` Send **403, Forbidden** Header
     *  `1` **Strip the POST/GET query** and forward the request! unset($_POST)

diff --git a/libs/config.sample.php b/libs/config.sample.php
@@ -2,14 +2,12 @@
 /**
  * Configuration file for CSRF Protector
  * Necessary configurations are (library would throw exception otherwise)
- * ---- logDirectory
  * ---- failedAuthAction
  * ---- jsUrl
  * ---- tokenLength
  */
 return array(
-	"CSRFP_TOKEN" => "",
-	"logDirectory" => "../log",
+    "CSRFP_TOKEN" => "",
 	"failedAuthAction" => array(
 		"GET" => 0,
 		"POST" => 0),
@@ -26,5 +24,5 @@
 	"disabledJavascriptMessage" => "This site attempts to protect users against <a href=\"https://www.owasp.org/index.php/Cross-Site_Request_Forgery_%28CSRF%29\">
 	Cross-Site Request Forgeries </a> attacks. In order to do so, you must have JavaScript enabled in your web browser otherwise this site will fail to work correctly for you.
 	 See details of your web browser for how to enable JavaScript.",
-	 "verifyGetFor" => array()
+	"verifyGetFor" => array()
 );
diff --git a/libs/csrf/LoggerInterface.php b/libs/csrf/LoggerInterface.php
@@ -1,6 +1,6 @@
 <?php
 /**
- * This file has implementation for LoggerInterface interface
+ * CSRF Protector's Logger interface.
  */
 
 if (!defined('__CSRF_PROTECTOR_LOGGER_INTERFACE__')) {

diff --git a/libs/csrf/csrfpCookieConfig.php b/libs/csrf/csrfpCookieConfig.php
@@ -35,7 +35,8 @@ class csrfpCookieConfig
 
         /**
          * Variable: $expire
-         * expiry parameter in seconds from now for setcookie method, default is 30 minutes
+         * expiry parameter in seconds from now for setcookie method, default is
+         *  30 minutes
          * @var int
          */
         public $expire = 1800;
@@ -48,10 +49,21 @@ class csrfpCookieConfig
          */
         function __construct($cfg) {
             if ($cfg !== null) {
-                if (isset($cfg['path'])) $this->path = $cfg['path'];
-                if (isset($cfg['domain'])) $this->domain = $cfg['domain'];
-                if (isset($cfg['secure'])) $this->secure = (bool) $cfg['secure'];
-                if (isset($cfg['expire']) && $cfg['expire']) $this->expire = (int)$cfg['expire'];
+                if (isset($cfg['path'])) {
+                    $this->path = $cfg['path'];
+                }
+
+                if (isset($cfg['domain'])) {
+                    $this->domain = $cfg['domain'];
+                }
+
+                if (isset($cfg['secure'])) {
+                    $this->secure = (bool) $cfg['secure'];
+                }
+
+                if (isset($cfg['expire']) && $cfg['expire']) {
+                    $this->expire = (int)$cfg['expire'];
+                }
             }
         }
     }

diff --git a/libs/csrf/csrfpDefaultLogger.php b/libs/csrf/csrfpDefaultLogger.php
@@ -8,82 +8,33 @@
     // to avoid multiple declaration errors
     define('__CSRF_PROTECTOR_DEFAULT_LOGGER_', true);
 
-    class logDirectoryNotFoundException extends \exception {};
-    class logFileWriteError extends \exception {};
-
     /**
      * Default logger class for CSRF Protector.
      * 
-     * This is a file based logger class.
+     * This implementation is based on PHP's default error_log implementation.
      */
     class csrfpDefaultLogger implements LoggerInterface {
-
         /**
-         * Variable: $logDirectory
-         * directory for file based logging
-         */
-        private $logDirectory;
-
-        /**
-         * Constructor
-         * 
-         * Parameters:
-         * $path - the path for logs to be stored (relative or absolute)
+         * Sends error message to the defined error_handling routines.
          * 
-         * Returns:
-         * void
-         *
-         * Throws:
-         * logDirectoryNotFoundException - if log directory is not found
-         */
-        function __construct($path) {
-            // Check for relative path
-            $this->logDirectory = __DIR__ . "/../" . $path;
-
-
-            // If the relative log directory path does not exist try as an absolute path.
-            if (!is_dir($this->logDirectory)) {
-                $this->logDirectory = $path;
-            }
-
-            if (!is_dir($this->logDirectory)) {
-                throw new logDirectoryNotFoundException("OWASP CSRFProtector: Log Directory Not Found!");
-            }
-        }
-
-        /**
-         * logging method
+         * Based on PHP's default error_log method implementation.
          *
          * Parameters:
          * $message - the log message
          * $context - context array
          * 
          * Return:
          * void
-         *
-         * Throws:
-         * logFileWriteError - if unable to log an attack
          */
         public function log($message, $context = array()) {
-            // Append to the log file, or create it if it does not exist create
-            $logFile = fopen($this->logDirectory ."/" . date("m-20y") . ".log", "a+");
-
-            // Throw exception if above fopen fails
-            if (!$logFile) {
-                throw new logFileWriteError("OWASP CSRFProtector: Unable to write to the log file");    
-            }
-
             $context['timestamp'] = time();
             $context['message'] = $message;
 
             // Convert log array to JSON format to be logged
-            $context = json_encode($context) .PHP_EOL;
-
-            // Append log to the file
-            fwrite($logFile, $context);
-
-            // Close the file handler
-            fclose($logFile);
+            $contextString = "OWASP CSRF Protector PHP " 
+                .json_encode($context) 
+                .PHP_EOL;
+            error_log($contextString, /* message_type= */ 0);
         }
     }
-}
+}
diff --git a/libs/csrf/csrfprotector.php b/libs/csrf/csrfprotector.php
@@ -10,7 +10,7 @@
 if (!defined('__CSRF_PROTECTOR__')) {
     define('__CSRF_PROTECTOR__', true);         // to avoid multiple declaration errors
 
-    // name of HTTP POST variable for authentication
+    // Name of HTTP POST variable for authentication
     define("CSRFP_TOKEN","CSRFP-Token");
 
     // We insert token name and list of url patterns for which
@@ -19,13 +19,13 @@
     define("CSRFP_FIELD_TOKEN_NAME", "csrfp_hidden_data_token");
     define("CSRFP_FIELD_URLS", "csrfp_hidden_data_urls");
 
-    /**
-     * child exception classes
-     */
+    /** Indicates configuration file was not found. */
     class configFileNotFoundException extends \exception {};
-    class jsFileNotFoundException extends \exception {};
-    class baseJSFileNotFoundExceptio extends \exception {};
+
+    /** Indicates that configuration file is incomplete. */
     class incompleteConfigurationException extends \exception {};
+
+    /** Indicates that CSRF Protector is already initialized. */
     class alreadyInitializedException extends \exception {};
 
     class csrfProtector
@@ -76,30 +76,38 @@ class csrfProtector
          * Variable: $config
          * config file for CSRFProtector
          * @var int Array, length = 6
-         * Property: #1: failedAuthAction (int) => action to be taken in case autherisation fails
-         * Property: #2: logDirectory (string) => directory in which log will be saved
-         * Property: #3: customErrorMessage (string) => custom error message to be sent in case
-         *                        of failed authentication
-         * Property: #4: jsFile (string) => location of the CSRFProtector js file
-         * Property: #5: tokenLength (int) => default length of hash
-         * Property: #6: disabledJavascriptMessage (string) => error message if client's js is disabled
+         * Property: #1: failedAuthAction (int) => action to be taken in case 
+         * autherisation fails.
+         * Property: #3: customErrorMessage (string) => custom error message to
+         * be sent in case of failed authentication.
+         * Property: #4: jsFile (string) => location of the CSRFProtector js
+         * file.
+         * Property: #5: tokenLength (int) => default length of hash.
+         * Property: #6: disabledJavascriptMessage (string) => error message if
+         * client's js is disabled.
+         * 
+         * TODO(mebjas): this field should be private
          */
         public static $config = array();
 
         /*
          * Variable: $requiredConfigurations
          * Contains list of those parameters that are required to be there
          *     in config file for csrfp to work
+         * 
+         * TODO(mebjas): this field should be private
          */
-        public static $requiredConfigurations  = array('logDirectory', 'failedAuthAction', 'jsUrl', 'tokenLength');
+        public static $requiredConfigurations  = array(
+            'failedAuthAction', 'jsUrl', 'tokenLength');
 
         /*
          * Function: function to initialise the csrfProtector work flow
          *
          * Parameters:
-         * $length - length of CSRF_AUTH_TOKEN to be generated
-         * $action - int array, for different actions to be taken in case of failed validation
-         * $logger - custom logger class object
+         * $length - (int) length of CSRF_AUTH_TOKEN to be generated.
+         * $action - (int array), for different actions to be taken in case of
+         *      failed validation.
+         * $logger - (LoggerInterface) custom logger class object.
          *
          * Returns:
          * void
@@ -179,11 +187,11 @@ public static function init($length = null, $action = null, $logger = null)
                     implode(', ', $missingConfiguration) . ' value(s)');
             }
 
-            // Iniialize the logger class
+            // Initialize the logger class
             if ($logger !== null) {
                 self::$logger = $logger;
             } else {
-                self::$logger = new csrfpDefaultLogger(self::$config['logDirectory']);
+                self::$logger = new csrfpDefaultLogger();
             }
 
             // Authorise the incoming request
@@ -212,9 +220,8 @@ public static function init($length = null, $action = null, $logger = null)
          *
          * Returns: 
          * void
-         *
-         * Throws: 
-         * logDirectoryNotFoundException - if log directory is not found
+         * 
+         * TODO(mebjas): this method should be private.
          */
         public static function authorizePost()
         {
@@ -539,7 +546,8 @@ protected static function logCSRFattack()
             $context['REQUEST_URI'] = $_SERVER['REQUEST_URI'];
             $context['requestType'] = self::$requestType;
             $context['cookie'] = $_COOKIE;
-            self::$logger->log("OWASP CSRF PROTECTOR VALIDATION FAILURE", $context);
+            self::$logger->log(
+                "OWASP CSRF PROTECTOR VALIDATION FAILURE", $context);
         }
 
         /*

diff --git a/log/.htaccess b/log/.htaccess
diff --git a/log/index.php b/log/index.php
diff --git a/readme.md b/readme.md
@@ -3,8 +3,7 @@ CSRF Protector
 [![Todo Status](http://todofy.org/b/mebjas/CSRF-Protector-PHP)](http://todofy.org/r/mebjas/CSRF-Protector-PHP) [![Build Status](https://travis-ci.org/mebjas/CSRF-Protector-PHP.svg?branch=master)](https://travis-ci.org/mebjas/CSRF-Protector-PHP) 
 <br>CSRF protector php, a standalone php library for csrf mitigation in web applications. Easy to integrate in any php web app. 
 
-Add to your project using packagist
-==========
+# Add to your project using packagist
  Add a `composer.json` file to your project directory
  ```json
  {
@@ -16,61 +15,60 @@ Add to your project using packagist
 Then open terminal (or command prompt), move to project directory and run
 ```shell
 composer install
-```
-OR
-```
+
+## Or alternatively
+
 php composer.phar install
 ```
-This will add CSRFP (library will be downloaded at ./vendor/owasp/csrf-protector-php) to your project directory. View [packagist.org](https://packagist.org/) for more help with composer!
+This will add CSRFP (library will be downloaded at `./vendor/owasp/csrf-protector-php`) to your project directory. View [packagist.org](https://packagist.org/) for more help with composer!
 
-Configuration
-==========
+# Configuration
 For composer installations: Copy the config.sample.php file into your root folder at config/csrf_config.php
 For non-composer installations: Copy the `libs/csrf/config.sample.php` file into `libs/csrf/config.php`
 Edit config accordingly. See Detailed Information link below.
 
 [Link to wiki - Editing Configurations & Mandatory requirements before using this library](https://github.com/mebjas/CSRF-Protector-PHP/wiki/Configurations)
 
-How to use
-==========
+# How to use
 ```php
 <?php
 include_once __DIR__ .'/vendor/owasp/csrf-protector-php/libs/csrf/csrfprotector.php';
 
-//Initialise CSRFGuard library
+// Initialise CSRFProtector library
 csrfProtector::init();
 ```
 simply include the library and call the `init()` function!
 
-### Detailed information @[Project wiki on github](https://github.com/mebjas/CSRF-Protector-PHP/wiki)
-
-### More information @[OWASP wiki](https://www.owasp.org/index.php/CSRFProtector_Project)
-
-### Contribute
+### More information 
+ - [Project wiki on Github](https://github.com/mebjas/CSRF-Protector-PHP/wiki)
+ - [OWASP wiki](https://www.owasp.org/index.php/CSRFProtector_Project)
 
-* Fork the repo
-* Create your branch
-* Commit your changes
-* Create a pull request
-
-### Note
-This version (`master`) requires the clients to have Javascript enabled. However if your application can work without javascript & you require a nojs version of this library, check our [nojs version](https://github.com/mebjas/CSRF-Protector-PHP/tree/nojs-support)
-
-## Discussion
+## Discussions
 Join Discussions at [Google Group \ OWASP \ CSRF Protector](https://groups.google.com/a/owasp.org/forum/#!forum/csrfprotector-project)
 
-~~Join Discussions on the [mailing list](https://lists.owasp.org/mailman/listinfo/owasp-csrfprotector)~~
-
-For any other queries contact me at: **[email protected]**
+For any other queries contact me at: [email protected] | [email protected]
 
 ## How to contribute?
-Well, there are various ways to contribute to this project. Find few of them listed below:
+### General steps
+ - Fork the repo
+ - Create your branch
+ - Commit your changes
+ - Create a pull request
+
+### More?
+Well, there are various ways to contribute to this project. Find a few of them listed below:
  - Found a bug? Raise a bug in [the issue page](https://github.com/mebjas/CSRF-Protector-PHP/issues?q=is%3Aissue+is%3Aopen+label%3Abug). Please make sure it's not a duplicate of an existing issue.
- - Have a feature request? Raise one at [the issue page](https://github.com/mebjas/CSRF-Protector-PHP/issues?q=is%3Aissue+is%3Aopen+label%3Aenhancement). As mentioned above please do a basic check if this `enhancement` exist in mentioned link.
+ - Have a feature request? Raise one at [the issue page](https://github.com/mebjas/CSRF-Protector-PHP/issues?q=is%3Aissue+is%3Aopen+label%3Aenhancement). As mentioned above please do a basic check if this `enhancement` exists in the mentioned link.
  - Want to contribute code to this project?
-   - Best way to start is by picking up one of [the issues with `Up For Grab` label](https://github.com/mebjas/CSRF-Protector-PHP/issues?q=is%3Aissue+is%3Aopen+label%3A%22Up+For+Grabs%22). Leave a comment, that you intend to help on this > fork > send a pull request to `master branch`.
+   - The best way to start is by picking up one of the existing [issues with `Up For Grab` label](https://github.com/mebjas/CSRF-Protector-PHP/issues?q=is%3Aissue+is%3Aopen+label%3A%22Up+For+Grabs%22). 
+   - Leave a comment, that you intend to help on this > then fork > and then send a pull request to `master branch`.
 
-### FAQ:
+## FAQ:
 1. What happens if token expires? - https://github.com/mebjas/CSRF-Protector-PHP/wiki/what-if-token-expires
-2. Secure flag in cookie? - https://github.com/mebjas/CSRF-Protector-PHP/issues/54
+2. Secure flag in a cookie? - https://github.com/mebjas/CSRF-Protector-PHP/issues/54
 3. NoJS support? - https://github.com/mebjas/CSRF-Protector-PHP/tree/nojs-support
+
+## Appendix
+
+### JS not supported?
+This version (in `master` branch) requires the clients to have Javascript enabled. However if your application can work without javascript & you require a nojs version of this library, check our nojs version