From 842f00f45212019a3b07f8d8dac269d35beb9efa Mon Sep 17 00:00:00 2001 From: Lukas Eipert Date: Wed, 30 Dec 2020 21:24:16 +0100 Subject: [PATCH] [ruby/net-http] Decode user and password from env configured proxy If someone sets an env variable defining a http_proxy, containing a username / password with percent-encoded characters, then the resulting base64 encoded auth header will be wrong. For example, suppose a username is `Y\X` and the password is `R%S] ?X`. Properly URL encoded the proxy url would be: http://Y%5CX:R%25S%5D%20%3FX@proxy.example:8000 The resulting proxy auth header should be: `WVxYOlIlU10gP1g=`, but the getters defined by ruby StdLib `URI` return a username `Y%5CX` and password `R%25S%5D%20%3FX`, resulting in `WSU1Q1g6UiUyNVMlNUQlMjAlM0ZY`. As a result the proxy will deny the request. Please note that this is my first contribution to the ruby ecosystem, to standard lib especially and I am not a ruby developer. References: - https://gitlab.com/gitlab-org/gitlab/-/issues/289836 - https://bugs.ruby-lang.org/projects/ruby-master/repository/trunk/revisions/58461 - https://bugs.ruby-lang.org/issues/17542 https://github.com/ruby/net-http/commit/e57d4f38aa --- lib/net/http.rb | 11 +++++++++-- test/net/http/test_http.rb | 17 +++++++++++++++++ 2 files changed, 26 insertions(+), 2 deletions(-) diff --git a/lib/net/http.rb b/lib/net/http.rb index 33f8b567bd..9351606215 100644 --- a/lib/net/http.rb +++ b/lib/net/http.rb @@ -1178,7 +1178,8 @@ module Net #:nodoc: # The username of the proxy server, if one is configured. def proxy_user if ENVIRONMENT_VARIABLE_IS_MULTIUSER_SAFE && @proxy_from_env - proxy_uri&.user + user = proxy_uri&.user + unescape(user) if user else @proxy_user end @@ -1187,7 +1188,8 @@ module Net #:nodoc: # The password of the proxy server, if one is configured. def proxy_pass if ENVIRONMENT_VARIABLE_IS_MULTIUSER_SAFE && @proxy_from_env - proxy_uri&.password + pass = proxy_uri&.password + unescape(pass) if pass else @proxy_pass end @@ -1198,6 +1200,11 @@ module Net #:nodoc: private + def unescape(value) + require 'cgi/util' + CGI.unescape(value) + end + # without proxy, obsolete def conn_address # :nodoc: diff --git a/test/net/http/test_http.rb b/test/net/http/test_http.rb index 22448d828f..60b6d51f99 100644 --- a/test/net/http/test_http.rb +++ b/test/net/http/test_http.rb @@ -188,6 +188,23 @@ class TestNetHTTP < Test::Unit::TestCase end end + def test_proxy_eh_ENV_with_urlencoded_user + TestNetHTTPUtils.clean_http_proxy_env do + ENV['http_proxy'] = 'http://Y%5CX:R%25S%5D%20%3FX@proxy.example:8000' + + http = Net::HTTP.new 'hostname.example' + + assert_equal true, http.proxy? + if Net::HTTP::ENVIRONMENT_VARIABLE_IS_MULTIUSER_SAFE + assert_equal "Y\\X", http.proxy_user + assert_equal "R%S] ?X", http.proxy_pass + else + assert_nil http.proxy_user + assert_nil http.proxy_pass + end + end + end + def test_proxy_eh_ENV_none_set TestNetHTTPUtils.clean_http_proxy_env do assert_equal false, Net::HTTP.new('hostname.example').proxy?