🔀 Merge pull request #209 from oauth-xx/issue/156-fix-unsafe-comparison

Issue/156 fix unsafe comparison

.rubocop_todo.yml

@@ -1,6 +1,6 @@
 # This configuration was generated by
 # `rubocop --auto-gen-config`
-# on 2021-10-31 17:21:56 UTC using RuboCop version 1.22.3.
+# on 2021-10-31 19:10:34 UTC using RuboCop version 1.22.3.
 # The point is for the user to remove these configuration records
 # one by one as the offenses are removed from the code base.
 # Note that changes in the inspected code, or installation of new
@@ -35,13 +35,12 @@ Layout/AccessModifierIndentation:
     - 'lib/oauth/tokens/request_token.rb'
     - 'test/cases/spec/1_0-final/test_parameter_encodings.rb'
 
-# Offense count: 16
+# Offense count: 12
 # Cop supports --auto-correct.
 # Configuration parameters: EnforcedStyle, IndentationWidth.
 # SupportedStyles: with_first_argument, with_fixed_indentation
 Layout/ArgumentAlignment:
   Exclude:
-    - 'lib/oauth/consumer.rb'
     - 'lib/oauth/server.rb'
     - 'test/units/test_em_http_request_proxy.rb'
     - 'test/units/test_rest_client_request_proxy.rb'
@@ -317,7 +316,7 @@ Layout/MultilineOperationIndentation:
   Exclude:
     - 'lib/oauth/consumer.rb'
 
-# Offense count: 202
+# Offense count: 183
 # Cop supports --auto-correct.
 Layout/SpaceAfterComma:
   Enabled: false
@@ -452,13 +451,12 @@ Layout/TrailingWhitespace:
   Exclude:
     - 'lib/oauth/request_proxy/rest_client_request.rb'
 
-# Offense count: 7
+# Offense count: 6
 # Cop supports --auto-correct.
 Lint/AmbiguousOperatorPrecedence:
   Exclude:
     - 'lib/oauth/cli/sign_command.rb'
     - 'lib/oauth/consumer.rb'
-    - 'test/test_helper.rb'
 
 # Offense count: 2
 # Configuration parameters: AllowSafeAssignment.
@@ -567,7 +565,7 @@ Metrics/AbcSize:
 # Offense count: 9
 # Configuration parameters: CountComments, CountAsOne.
 Metrics/ClassLength:
-  Max: 274
+  Max: 277
 
 # Offense count: 7
 # Configuration parameters: IgnoredMethods.
@@ -1196,15 +1194,14 @@ Style/StderrPuts:
   Exclude:
     - 'lib/oauth/request_proxy/base.rb'
 
-# Offense count: 17
+# Offense count: 16
 # Cop supports --auto-correct.
 # Configuration parameters: Mode.
 Style/StringConcatenation:
   Exclude:
     - 'lib/oauth/cli/sign_command.rb'
     - 'lib/oauth/client/net_http.rb'
     - 'test/integration/consumer_test.rb'
-    - 'test/test_helper.rb'
     - 'test/units/test_net_http_client.rb'
     - 'test/units/test_rsa_sha1.rb'
 

lib/oauth/consumer.rb

@@ -157,11 +157,14 @@ def get_request_token(request_options = {}, *arguments, &block)
       request_options[:oauth_callback] ||= OAuth::OUT_OF_BAND unless request_options[:exclude_callback]
 
       if block_given?
-        response = token_request(http_method,
-        (request_token_url? ? request_token_url : request_token_path),
-        nil,
-        request_options,
-        *arguments, &block)
+        response = token_request(
+          http_method,
+          (request_token_url? ? request_token_url : request_token_path),
+          nil,
+          request_options,
+          *arguments,
+          &block
+        )
       else
         response = token_request(http_method, (request_token_url? ? request_token_url : request_token_path), nil, request_options, *arguments)
       end

lib/oauth/signature/base.rb

@@ -51,7 +51,9 @@ def signature
     end
 
     def ==(cmp_signature)
-      signature == cmp_signature
+      check = signature.bytesize ^ cmp_signature.bytesize
+      signature.bytes.zip(cmp_signature.bytes) { |x, y| check |= x ^ y.to_i }
+      check.zero?
     end
 
     def verify

test/integration/consumer_test.rb

@@ -138,7 +138,7 @@ def test_step_by_step_token_request
       assert_equal "GET", request.method
       assert_nil request.body
       response=@consumer.http.request(request)
-      assert_equal "200",response.code
+      assert_equal "200", response.code
       assert_equal "oauth_token=requestkey&oauth_token_secret=requestsecret",response.body
     end
 
@@ -163,24 +163,24 @@ def test_get_token_sequence
 
       @request_token=@consumer.get_request_token
       assert @request_token
-      assert_equal "requestkey",@request_token.token
-      assert_equal "requestsecret",@request_token.secret
+      assert_equal "requestkey", @request_token.token
+      assert_equal "requestsecret", @request_token.secret
       assert_equal "http://term.ie/oauth/example/authorize.php?oauth_token=requestkey",@request_token.authorize_url
 
       @access_token=@request_token.get_access_token
       assert @access_token
-      assert_equal "accesskey",@access_token.token
-      assert_equal "accesssecret",@access_token.secret
+      assert_equal "accesskey", @access_token.token
+      assert_equal "accesssecret", @access_token.secret
 
       @response=@access_token.get("/oauth/example/echo_api.php?ok=hello&test=this")
       assert @response
-      assert_equal "200",@response.code
-      assert_equal( "ok=hello&test=this",@response.body)
+      assert_equal "200", @response.code
+      assert_equal( "ok=hello&test=this", @response.body)
 
       @response=@access_token.post("/oauth/example/echo_api.php",{"ok"=>"hello","test"=>"this"})
       assert @response
-      assert_equal "200",@response.code
-      assert_equal( "ok=hello&test=this",@response.body)
+      assert_equal "200", @response.code
+      assert_equal( "ok=hello&test=this", @response.body)
     end
 
     def test_get_token_sequence_using_fqdn
@@ -195,33 +195,33 @@ def test_get_token_sequence_using_fqdn
           :access_token_url=>"http://term.ie/oauth/example/access_token.php",
           :authorize_url=>"http://term.ie/oauth/example/authorize.php"
           })
-      assert_equal "http://term.ie/oauth/example/request_token.php",@consumer.request_token_url
-      assert_equal "http://term.ie/oauth/example/access_token.php",@consumer.access_token_url
+      assert_equal "http://term.ie/oauth/example/request_token.php", @consumer.request_token_url
+      assert_equal "http://term.ie/oauth/example/access_token.php", @consumer.access_token_url
 
       assert @consumer.request_token_url?, "Should use fully qualified request token url"
       assert @consumer.access_token_url?, "Should use fully qualified access token url"
       assert @consumer.authorize_url?, "Should use fully qualified url"
 
       @request_token=@consumer.get_request_token
       assert @request_token
-      assert_equal "requestkey",@request_token.token
-      assert_equal "requestsecret",@request_token.secret
-      assert_equal "http://term.ie/oauth/example/authorize.php?oauth_token=requestkey",@request_token.authorize_url
+      assert_equal "requestkey", @request_token.token
+      assert_equal "requestsecret", @request_token.secret
+      assert_equal "http://term.ie/oauth/example/authorize.php?oauth_token=requestkey", @request_token.authorize_url
 
       @access_token=@request_token.get_access_token
       assert @access_token
-      assert_equal "accesskey",@access_token.token
-      assert_equal "accesssecret",@access_token.secret
+      assert_equal "accesskey", @access_token.token
+      assert_equal "accesssecret", @access_token.secret
 
       @response=@access_token.get("/oauth/example/echo_api.php?ok=hello&test=this")
       assert @response
-      assert_equal "200",@response.code
-      assert_equal( "ok=hello&test=this",@response.body)
+      assert_equal "200", @response.code
+      assert_equal( "ok=hello&test=this", @response.body)
 
       @response=@access_token.post("/oauth/example/echo_api.php",{"ok"=>"hello","test"=>"this"})
       assert @response
       assert_equal "200",@response.code
-      assert_equal( "ok=hello&test=this",@response.body)
+      assert_equal( "ok=hello&test=this", @response.body)
     end