1. 13
  1.  

  2. 1

    How is this a Rails issue?

    1. 1

      Because Rails lets you send integers directly to MySQL by way of XML or YAML. In most other scenarios, every piece of data from the user is a string, which, when passed to MySQL, doesn’t have (m)any weird effects. Passing an integer directly to MySQL has some unexpected results, like NULL being equal to 0 (but not = ‘0’).

      mysql> select count(id) from users where email_verification_token = '';
      +-----------+
      | count(id) |
      +-----------+
      |         0 |
      +-----------+
      1 row in set (0.03 sec)
      
      mysql> select count(id) from users where email_verification_token = '0';
      +-----------+
      | count(id) |
      +-----------+
      |         0 |
      +-----------+
      1 row in set (0.02 sec)
      
      mysql> select count(id) from users where email_verification_token = 0;
      +-----------+
      | count(id) |
      +-----------+
      |       464 |
      +-----------+
      1 row in set (0.03 sec)
      

      By passing XML/YAML to Rails, you can get an actual integer value in params. If params[:token] is a string of “0”, this:

       User.find_by_password_reset_token(params[:token])
      

      is executing:

        User Load (3.0ms)  SELECT `users`.* FROM `users` WHERE `users`.`password_reset_token` = '0' LIMIT 1 
      => nil
      

      but when it’s an actual 0, it becomes:

      irb(main):002:0> User.find_by_password_reset_token(0)
        User Load (0.2ms)  SELECT `users`.* FROM `users` WHERE `users`.`password_reset_token` = 0 LIMIT 1
      => #<User id: ...
      

      which returns the first user with a null password reset token.

      For what it’s worth, all of this came straight from the Lobste.rs console. This problem would have been exploitable on this site’s password reset page but I’ve been running with:

      ActionDispatch::ParamsParser::DEFAULT_PARSERS = {}
      

      on all of my Rails applications since the first YAML vulnerability was announced. None of my apps need to parse incoming XML or YAML or any other stupid stuff.

      1. 1

        “Because Rails lets you send integers directly to MySQL by way of XML or YAML. In most other scenarios, every piece of data from the user is a string, which, when passed to MySQL, doesn’t have (m)any weird effects. Passing an integer directly to MySQL has some unexpected results, like NULL being equal to 0 (but not = ‘0’).”

        Do people write apps where they allow user-provided data to pass-through unchecked? If true, is that the fault of the framework?

        Am I misunderstanding something here? This looks more like poor security practices coupled with goofy database behavior, compounded simply because Rails makes it easier to do.

        1. 1

          Do people write apps where they allow user-provided data to pass-through unchecked?

          Yes, because the data is properly escaped, so there is (usually) no harm in passing it through to SQL. That’s often how the checking is done — whether SQL returns records or not. If you have to double up each check with a regex or some kind of pattern match first before sending it to SQL, it becomes pretty tedious and error-prone.

          One way around it is to explicitly cast everything to a string before doing anything with it, but the framework should really not be allowing other stuff in there to begin with.

          I should mention that this particular piece of code from Lobsters is written like this:

          if params[:token].blank? ||
          !(@reset_user = User.find_by_password_reset_token(params[:token]))
            [...]
            return redirect_to forgot_password_url
          end
          

          so it makes sure that params[:token] is not blank before passing it to SQL. Since it’s not null (or a blank string), it should stand to reason that any other string is ok to pass to SQL as a possible password reset token. However, because of this XML problem, it can be an integer 0.

          irb(main):001:0> "".blank?
          => true
          irb(main):002:0> 0.blank?
          => false
          

          So now it bypasses the blank? check and gets passed to SQL directly as a 0, which compares to NULLs which are casted to integers (becoming zeros), and matches the first user record.