weixin_39593427
weixin_39593427
2020-11-30 11:34

Password displayed in cleartext on Result instance

Hey there, question for the maintainers. I recently used the Mysql2::Client class to fetch some results from my app's database, and I discovered that the database password is displayed in cleartext in the Mysql2::Result instance's query_options instance variable. I reproduced the issue with a throwaway Rails app below:


irb(main):004:0> client = Mysql2::Client.new(:host => "localhost", :username => "foobar", password: 'xyz')

=> #<:client:0x00007f9ca0aac398>:hash, :async=>false, :cast_booleans=>false, :symbolize_keys=>false, :database_timezone=>:local, :application_timezone=>nil, :cache_rows=>true, :connect_flags=>2148540933, :cast=>true, :default_file=>nil, :default_group=>nil, :host=>"localhost", :username=>"foobar", :password=>"xyz"}>

irb(main):005:0> client.select_db('tempapp_development')

=> "tempapp_development"

irb(main):006:0> results = client.query("SELECT * FROM foobars WHERE name='foo'")                         
=> #<:result:0x00007f9ca0a761f8>:hash, :async=>false, :cast_booleans=>false, :symbolize_keys=>false, :database_timezone=>:local, :application_timezone=>nil, :cache_rows=>true, :connect_flags=>2148540933, :cast=>true, :default_file=>nil, :default_group=>nil, :host=>"localhost", :username=>"foobar", :password=>"xyz"}, ={:no_good_index_used=>false, :no_index_used=>true, :query_was_slow=>false}>

irb(main):007:0> pp _

#<:result:0x00007f9ca0a761f8>:hash,
   :async=>false,
   :cast_booleans=>false,
   :symbolize_keys=>false,
   :database_timezone=>:local,
   :application_timezone=>nil,
   :cache_rows=>true,
   :connect_flags=>2148540933,
   :cast=>true,
   :default_file=>nil,
   :default_group=>nil,
   :host=>"localhost",
   :username=>"foobar",
   :password=>"xyz"},            # <= cleartext password here
 =
  {:no_good_index_used=>false, :no_index_used=>true, :query_was_slow=>false}>

=> #<:result:0x00007f9ca0a761f8>:hash, :async=>false, :cast_booleans=>false, :symbolize_keys=>false, :database_timezone=>:local, :application_timezone=>nil, :cache_rows=>true, :connect_flags=>2148540933, :cast=>true, :default_file=>nil, :default_group=>nil, :host=>"localhost", :username=>"foobar", :password=>"xyz"}, ={:no_good_index_used=>false, :no_index_used=>true, :query_was_slow=>false}>
</:result:0x00007f9ca0a761f8></:result:0x00007f9ca0a761f8></:result:0x00007f9ca0a761f8></:client:0x00007f9ca0aac398>

I'm curious as to why this parameter is displayed in cleartext, as opposed to obfuscated when output to the Rails console.

Related question- it makes sense why it's listed among the `` of the Client class, but I'm curious why it's also listed on the Result class.

I trust there's a valid reason, and as a journeyman engineer I'm seeking to learn what that is, to better understand how this gem works. Thanks for building and maintaining this gem, I can only imagine how work it is to manage an open-source project of this scale.

该提问来源于开源项目:brianmario/mysql2

  • 点赞
  • 写回答
  • 关注问题
  • 收藏
  • 复制链接分享
  • 邀请回答

5条回答

  • weixin_39593427 weixin_39593427 5月前

    Follow-up question- would y'all accept a PR which overrides the inspect methods for the Client and Result class to something like the following:

    
    <:result:0x00007f9ca0a761f8>:hash,
       :async=>false,
       :cast_booleans=>false,
       :symbolize_keys=>false,
       :database_timezone=>:local,
       :application_timezone=>nil,
       :cache_rows=>true,
       :connect_flags=>2148540933,
       :cast=>true,
       :default_file=>nil,
       :default_group=>nil,
       :host=>"localhost",
       :username=>"foobar",
       :password=>[FILTERED]},      # <= 'query_options' itself is unaffected, but redacted when Result is stringified
     =
      {:no_good_index_used=>false, :no_index_used=>true, :query_was_slow=>false}>
    </:result:0x00007f9ca0a761f8>

    This issue actually came up in our production codebase recently, where an engineer copy/pasted a Result object from the prod database into a GitHub comment without realizing that it contained the cleartext password. This in turn triggered emails which included the comment (and therefore the cleartext prod DB password) to be sent to multiple people watching the GH repo.

    I could also see this issue coming up if someone's repo is outputting these two classes to a logfile or 3rd-party logging service somewhere.

    Assuming there are no dependencies on the string-ified classes containing the password (and I can't imagine there would be), it seems like a PR to address this would be low-hanging fruit.

    点赞 评论 复制链接分享
  • weixin_39564617 weixin_39564617 5月前

    Aha, great explanation of the problem. Changing a password is not useful after the initial connection. I think the sensible thing to do is have the C code internalize the password, or even remove it from memory entirely, once it's part of the MySQL connection structure.

    Rather than overriding #inspect, I'm thinking to redact or even remove the password field from the struct entirely.

    点赞 评论 复制链接分享
  • weixin_39593427 weixin_39593427 5月前

    I think the sensible thing to do is have the C code internalize the password, or even remove it from memory entirely, once it's part of the MySQL connection structure.

    I'm not a C engineer so I don't have context on the time it would take to accomplish this. Is this issue time sensitive, given its security implications? And if so, would overriding inspect be a viable short-term workaround until the solution you mentioned is implemented? I'm not married to the solution I suggested by any means, just wondering if a two-part, "immediate hotfix" + "long-term solution" would be useful in this case.

    点赞 评论 复制链接分享
  • weixin_39593427 weixin_39593427 5月前

    Also, in retrospect- I feel I should have submitted this issue as a private message or otherwise followed Responsible Disclosure guidelines. My apologies if that's the preferred path for issues like this. Would emailing you directly on the email on your GH profile have been appropriate in this case?

    点赞 评论 复制链接分享
  • weixin_39564617 weixin_39564617 5月前

    I'm not a C engineer so I don't have context on the time it would take to accomplish this.

    Not hard.

    Is this issue time sensitive, given its security implications.

    No.

    I feel I should have submitted this issue as a private message or otherwise followed Responsible Disclosure guidelines.

    In this case no. The password is not leaked externally, and not based on input from an external user. This is in the realm of user error.

    That said, I consider this user error to be due to a "sharp edge" on this tool, and will be glad to prioritize filing off this sharp edge.

    点赞 评论 复制链接分享

相关推荐