SQL injection search

I can definitely see the issue if the server saves the unfiltered input and tries to print that out for other uses, but it seems to me that outputting a raw $_GET variable will only go to the requester and therefore could only run unfiltered code on that requesters machine.

EDIT: Answering my own question:

This is a security hole because the unsafe JavaScript is stored in the URL for the page (it is a GET parameter). This bad URL could then be sent as a link in a spam email or similar. Victims clicking on the link would then see a page that comes from the legitimate source, but is running unsafe code compromising that user's session. This sort of attack relies on the attacker distributing the link with the bad code as a URL parameter, and is not a vulnerability that a user could encounter when just visiting that site as I had first assumed.

https://github.com/search?p=4&q=extension%3Aphp+%3C%3F%3...

https://codeclimate.com/ is one I've used but it's Ruby only AFAIK.

Nothing new here. Google code search a while back was similar. Also search for index.php~, a great way to see the source for an index page with a unix temp file that Apache will forward as plain text. Oops.

    use PDO;
    use PDOException;

    /**
     * Used for interacting with the database. Usage:
     * <pre>
     * $db = Database::get();
     * $db->call( ... );
     * </pre>
     */
    class Database extends Obj {
      private static $instance;
      private $dataStore;

      /**
       * Sets the connection that this class uses for database transactions.
       */
      public function __construct() {
        global $dbhost;
        global $dbname;
        global $dbuser;
        global $dbpass;

        try {
          $this->setDataStore(
            new PDO( "pgsql:dbname=$dbname;host=$dbhost", $dbuser, $dbpass ) );
        }
        catch( PDOException $ex ) {
          $this->log( $ex->getMessage() );
        }
      }

      /**
       * Returns the singleton database instance.
       */
      public function get() {
        if( self::$instance === null ) {
          self::$instance = new Database();
        }

        return self::$instance;
      }

      /**
       * Call a database function and return the results. If there are
       * multiple columns to return, then the value for $params must contain
       * a comma; otherwise, without a comma, the value for $params is used
       * as the return column name. For example:
       *
       *- SELECT $params FROM $proc( ?, ? ); -- with comma
       *- SELECT $proc( ?, ? ) AS $params; -- without comma
       *- SELECT $proc( ?, ? ); -- empty
       *
       * @param $proc Name of the function or stored procedure to call.
       * @param $params Name of parameters to use as return columns.
       */
      public function call( $proc, $params = "" ) {
        $args = array();
        $count = 0;
        $placeholders = "";

        // Key is zero-based (e.g., $proc = 0, $params = 1).
        foreach( func_get_args() as $key => $parameter ) {
          // Skip the $proc and $params arguments to this method.
          if( $key < 2 ) continue;

          $count++;
          $placeholders = empty( $placeholders ) ? "?" : "$placeholders,?";
          array_push( $args, $parameter );
        }

        $sql = "";

        if( empty( $params ) ) {
          // If there are no parameters, then just make a call.
          $sql = "SELECT recipe.$proc( $placeholders )";
        }
        else if( strpos( $params, "," ) !== false ) {
          // If there is a comma, select the column names.
          $sql = "SELECT $params FROM recipe.$proc( $placeholders )";
        }
        else {
          // Otherwise, select the result into the given column name.
          $sql = "SELECT recipe.$proc( $placeholders ) AS $params";
        }

        $statement = $this->getDataStore()->prepare( $sql );

        //$this->log( "SQL: $sql" );

        for( $i = 1; $i <= $count; $i++ ) {
          //$this->log( "Bind " . $i . " to " . $args[$i - 1] );
          $statement->bindParam( $i, $args[$i - 1] );
        }

        $statement->execute();
        $result = $statement->fetchAll();
        $this->decodeArray( $result );

        return $result;
      }

      /**
       * Converts an array of numbers into an array suitable for usage with
       * PostgreSQL.
       *
       * @param $array An array of integers.
       */
      public function arrayToString( $array ) {
        return "{" . implode( ",", $array ) . "}";
      }

      /**
       * Recursive method to decode a UTF8-encoded array.
       *
       * @param $array - The array to decode.
       * @param $key - Name of the function to call.
       */
      private function decodeArray( &$array ) {
        if( is_array( $array ) ) {
          array_map( array( $this, "decodeArray" ), $array );
        }
        else {
          $array = utf8_decode( $array );
        }
      }

      private function getDataStore() {
        return $this->dataStore;
      }

      private function setDataStore( $dataStore ) {
        $this->dataStore = $dataStore;
      }
    }

    $db = Database::get();
    $result = $db->call( "is_existing_cookie", "existing", $cookie_value );

    return isset( $result[0] ) ? $result[0]["existing"] > 0 : false;

    private function authenticate() {
      $db = Database::get();
      $db->call( "authentication_upsert", "",
        $this->getCookieToken(),
        $this->getBrowserPlatform(),
        $this->getBrowserName(),
        $this->getBrowserVersion(),
        $this->getIp()
      );
    }

http://codereview.stackexchange.com/questions/26507/generic-...

https://github.com/search?p=2&q=MD5+password+extension%3...

https://github.com/search?q=CURLOPT_SSL_VERIFYHOST+NOT+depre...

There's more low-hanging fruit, if you're willing to use more specialized searches. For example, guess what mode of operation the PyCrypto library uses by default for all its block ciphers if you don't explicitly pick a sane one:

https://github.com/search?p=1&q=extension%3Apy+crypto+ci...

You don't even have to be a script kiddie. You just have to be able to click a button.

While there's nothing technically wrong with the example given, I might argue that since that won't work in all cases, it might be better to enforce a more rigorous policy of SQL query cleansing, or using bound params. Although this example is so simple I might not.

Then again, the fact that $_GET is even available at the location the query is taking place means this is most likely a type of design that I abhor, that PHP makes easy. Put actions in functions or methods, and then call them.

Casting to int is not a general purpose escaping system, and further, if you miss even one of these your entire application can be trashed.

Using mysql_query at all is a sign there's something severely wrong with your application.

Info about the QA/Security Consulting: http://www.cigital.com/services/

Secure coding plugin - http://www.cigital.com/products/secureassist/

When dealing with legacy code, especially code going back into the 1980s for example, it's not at all unusual for the code to have been stored in several different version control systems over that time. I know of projects that started with SCCS, moved to RCS, then to CVS, then to Perforce, then to Subversion, and most recently to Git.

If those transitions are done quickly, then somebody will often just take all of the code from a checkout of the old VCS, and check it into the new system.

The same can happen when initially using a VCS, after having not used one before, especially when taking a project over from a developer or even a team that was less-talented. Even if the new developer(s) are going to review the code, and fix bugs and other flaws, a version with the initial state of the code is often a useful thing to have.

http://pentestlab.wordpress.com/2012/11/27/automated-source-...

https://www.owasp.org/index.php/Static_Code_Analysis

http://code.google.com/p/yara-project/

http://ulsrl.org/

http://www.lightbluetouchpaper.org/ (the 3rd post

(these aren't the services, these are what you shd read to decide if some service's operatives are appropriately expensive, and up on current research

http://brakemanscanner.org/

Without some real-world context for each project, we just can't say whether the code we're looking at represents a real security problem. I'm certain that github hosts just as many one-off, let-me-scratch-some-code-together-that-I'll-never-use-again type projects as it does hard core, production quality ones.

That said it's a neat technique for quickly auditing code. Someone should now write an automated tool for submitting security patches to all of these projects.