Ruby: Add `rb/clear-text-logging-sensitive-data` query #7713

alexrford · 2022-01-23T22:56:28Z

Tracks cases where potentially sensitive data (e.g. passwords and other credentials) are logged as cleartext.

This is loosely based on the JS version of the same query, using the same SensitiveDataHeuristics.qll and a similar structure for the CleartextLoggingCustomizations.qll.

github-actions · 2022-01-28T17:10:58Z

QHelp previews:

ruby/ql/src/queries/security/cwe-312/CleartextLogging.qhelp

Clear-text logging of sensitive information

Sensitive information that is stored unencrypted is accessible to an attacker who gains access to the storage.

Recommendation

Ensure that sensitive information is always encrypted before being stored.

In general, decrypt sensitive information only at the point where it is necessary for it to be used in cleartext.

Be aware that external processes often store the standard out and standard error streams of the application, causing logged sensitive information to be stored as well.

Example

The following example code logs user credentials (in this case, their password) to standard out in plaintext:

require 'Logger'

class UserSession
  @@logger = Logger.new STDOUT

  def login(username, password)
    # ...
    @@logger.info "login from (user: #{username}, password: #{password})"
  end
end

Instead, the credentials should be masked or redacted before logging:

require 'Logger'

class UserSession
  @@logger = Logger.new STDOUT

  def login(username, password)
    # ...
    password_escaped = password.sub(/.*/, "[redacted]")
    @@logger.info "login from (user: #{username}, password: #{password_escaped})"
  end
end

References

M. Dowd, J. McDonald and J. Schuhm, The Art of Software Security Assessment, 1st Edition, Chapter 2 - 'Common Vulnerabilities of Encryption', p. 43. Addison Wesley, 2006.
M. Howard and D. LeBlanc, Writing Secure Code, 2nd Edition, Chapter 9 - 'Protecting Secret Data', p. 299. Microsoft, 2002.
Common Weakness Enumeration: CWE-312.
Common Weakness Enumeration: CWE-359.
Common Weakness Enumeration: CWE-532.

github-actions · 2022-01-28T17:13:43Z

QHelp previews:

ruby/ql/src/queries/security/cwe-312/CleartextLogging.qhelp

Clear-text logging of sensitive information

Sensitive information that is stored unencrypted is accessible to an attacker who gains access to the storage.

Recommendation

Ensure that sensitive information is always encrypted before being stored.

In general, decrypt sensitive information only at the point where it is necessary for it to be used in cleartext.

Be aware that external processes often store the standard out and standard error streams of the application, causing logged sensitive information to be stored as well.

Example

The following example code logs user credentials (in this case, their password) to standard out in plaintext:

require 'Logger'

class UserSession
  @@logger = Logger.new STDOUT

  def login(username, password)
    # ...
    @@logger.info "login from (user: #{username}, password: #{password})"
  end
end

Instead, the credentials should be masked or redacted before logging:

require 'Logger'

class UserSession
  @@logger = Logger.new STDOUT

  def login(username, password)
    # ...
    password_escaped = password.sub(/.*/, "[redacted]")
    @@logger.info "login from (user: #{username}, password: #{password_escaped})"
  end
end

References

M. Dowd, J. McDonald and J. Schuhm, The Art of Software Security Assessment, 1st Edition, Chapter 2 - 'Common Vulnerabilities of Encryption', p. 43. Addison Wesley, 2006.
M. Howard and D. LeBlanc, Writing Secure Code, 2nd Edition, Chapter 9 - 'Protecting Secret Data', p. 299. Microsoft, 2002.
Common Weakness Enumeration: CWE-312.
Common Weakness Enumeration: CWE-359.
Common Weakness Enumeration: CWE-532.

github-actions · 2022-01-28T17:29:39Z

QHelp previews:

ruby/ql/src/queries/security/cwe-312/CleartextLogging.qhelp

Clear-text logging of sensitive information

Sensitive information that is stored unencrypted is accessible to an attacker who gains access to the storage.

Recommendation

Ensure that sensitive information is always encrypted before being stored.

In general, decrypt sensitive information only at the point where it is necessary for it to be used in cleartext.

Be aware that external processes often store the standard out and standard error streams of the application, causing logged sensitive information to be stored as well.

Example

The following example code logs user credentials (in this case, their password) to standard out in plaintext:

require 'Logger'

class UserSession
  @@logger = Logger.new STDOUT

  def login(username, password)
    # ...
    @@logger.info "login with password: #{password})"
  end
end

Instead, the credentials should be masked or redacted before logging:

require 'Logger'

class UserSession
  @@logger = Logger.new STDOUT

  def login(username, password)
    # ...
    password_escaped = password.sub(/.*/, "[redacted]")
    @@logger.info "login with password: #{password_escaped})"
  end
end

References

M. Dowd, J. McDonald and J. Schuhm, The Art of Software Security Assessment, 1st Edition, Chapter 2 - 'Common Vulnerabilities of Encryption', p. 43. Addison Wesley, 2006.
M. Howard and D. LeBlanc, Writing Secure Code, 2nd Edition, Chapter 9 - 'Protecting Secret Data', p. 299. Microsoft, 2002.
Common Weakness Enumeration: CWE-312.
Common Weakness Enumeration: CWE-359.
Common Weakness Enumeration: CWE-532.

github-actions · 2022-01-28T17:39:50Z

QHelp previews:

ruby/ql/src/queries/security/cwe-312/CleartextLogging.qhelp

Clear-text logging of sensitive information

Sensitive information that is stored unencrypted is accessible to an attacker who gains access to the storage.

Recommendation

Ensure that sensitive information is always encrypted before being stored.

In general, decrypt sensitive information only at the point where it is necessary for it to be used in cleartext.

Be aware that external processes often store the standard out and standard error streams of the application, causing logged sensitive information to be stored as well.

Example

The following example code logs user credentials (in this case, their password) to standard out in plaintext:

require 'Logger'

class UserSession
  @@logger = Logger.new STDOUT

  def login(username, password)
    # ...
    @@logger.info "login with password: #{password})"
  end
end

Instead, the credentials should be masked or redacted before logging:

require 'Logger'

class UserSession
  @@logger = Logger.new STDOUT

  def login(username, password)
    # ...
    password_escaped = password.sub(/.*/, "[redacted]")
    @@logger.info "login with password: #{password_escaped})"
  end
end

References

M. Dowd, J. McDonald and J. Schuhm, The Art of Software Security Assessment, 1st Edition, Chapter 2 - 'Common Vulnerabilities of Encryption', p. 43. Addison Wesley, 2006.
M. Howard and D. LeBlanc, Writing Secure Code, 2nd Edition, Chapter 9 - 'Protecting Secret Data', p. 299. Microsoft, 2002.
Common Weakness Enumeration: CWE-312.
Common Weakness Enumeration: CWE-359.
Common Weakness Enumeration: CWE-532.

ruby/ql/lib/codeql/ruby/security/CleartextLoggingCustomizations.qll

hmac

I've left a few minor comments and questions. In general this looks super thorough, but I have trouble following some of the logic in CleartextLoggingCustomizations.qll, I think because we deal with heuristics both for "sensitive" data and "not sensitive" data. Hopefully a second read through will make it a bit clearer for me.

ruby/ql/lib/codeql/ruby/security/CleartextLoggingCustomizations.qll

hmac · 2022-02-03T01:36:37Z

ruby/ql/lib/codeql/ruby/security/CleartextLoggingCustomizations.qll

+   */
+  private class ObfuscatorCall extends Sanitizer, DataFlow::CallNode {
+    ObfuscatorCall() {
+      this.getMethodName().regexpMatch(notSensitiveRegexp()) and


I don't know if we can use notSensitiveRegexp as-is. It seems to flag any method with a ! in it, which is of course a common suffix for Ruby method names.

I encountered similar problems with [] and []= which I ended up special-casing using nonSensitiveMethodNameExclusion. The problem here is the [^\\w$.-] sub-clause that marks most methods containing non-alphanumerics as being "not sensitive".

In this case I think it could be sufficient to have ! as a suffix as another special exception for Ruby (and also to use nonSensitiveMethodNameExclusion more consistently in this file)? The other common naming pattern that I can think of is a ? suffix for predicate methods, which probably make sense as being marked as non-sensitive.

ruby/ql/lib/codeql/ruby/security/CleartextLoggingCustomizations.qll

ruby/ql/lib/codeql/ruby/security/internal/SensitiveDataHeuristics.qll

hmac · 2022-02-03T23:22:51Z

ruby/ql/lib/codeql/ruby/security/CleartextLoggingQuery.qll

+  override predicate isSink(DataFlow::Node sink) { sink instanceof CleartextLogging::Sink }
+
+  override predicate isSanitizer(DataFlow::Node node) {
+    super.isSanitizer(node)


TaintTracking::Configuration.isSanitizer(_) is none(), so isn't this line redundant?

I added it in case we extend the default isSanitizer predicate in the future, though I haven't been consistent in doing this with all dataflow/taint tracking queries.

hmac · 2022-02-03T23:27:15Z

ruby/ql/lib/codeql/ruby/security/CleartextLoggingCustomizations.qll

+  /** Holds if `nodeFrom` taints `nodeTo`. */
+  predicate isAdditionalTaintStep(DataFlow::Node nodeFrom, DataFlow::Node nodeTo) {
+    exists(string name, ElementReference ref, LocalVariable hashVar |
+      // from `hsh[password] = "changeme"` to a `hsh[password]` read


Can this additional step be dropped once we support flow through hashes?

Yes, I expect so.

ruby/ql/test/query-tests/security/cwe-312/logging.rb

Co-authored-by: Harry Maclean <[email protected]>

github-actions · 2022-02-08T18:04:27Z

QHelp previews:

ruby/ql/src/queries/security/cwe-312/CleartextLogging.qhelp

Clear-text logging of sensitive information

Sensitive information that is stored unencrypted is accessible to an attacker who gains access to the storage.

Recommendation

Ensure that sensitive information is always encrypted before being stored.

In general, decrypt sensitive information only at the point where it is necessary for it to be used in cleartext.

Be aware that external processes often store the standard out and standard error streams of the application, causing logged sensitive information to be stored as well.

Example

The following example code logs user credentials (in this case, their password) to standard out in plaintext:

require 'Logger'

class UserSession
  @@logger = Logger.new STDOUT

  def login(username, password)
    # ...
    @@logger.info "login with password: #{password})"
  end
end

Instead, the credentials should be masked or redacted before logging:

require 'Logger'

class UserSession
  @@logger = Logger.new STDOUT

  def login(username, password)
    # ...
    password_escaped = password.sub(/.*/, "[redacted]")
    @@logger.info "login with password: #{password_escaped})"
  end
end

References

M. Dowd, J. McDonald and J. Schuhm, The Art of Software Security Assessment, 1st Edition, Chapter 2 - 'Common Vulnerabilities of Encryption', p. 43. Addison Wesley, 2006.
M. Howard and D. LeBlanc, Writing Secure Code, 2nd Edition, Chapter 9 - 'Protecting Secret Data', p. 299. Microsoft, 2002.
Common Weakness Enumeration: CWE-312.
Common Weakness Enumeration: CWE-359.
Common Weakness Enumeration: CWE-532.

…r than AST nodes

…re accurately

… sensitive data)

…swordFlow

github-actions · 2022-02-10T16:48:49Z

QHelp previews:

ruby/ql/src/queries/security/cwe-312/CleartextLogging.qhelp

Clear-text logging of sensitive information

Sensitive information that is stored unencrypted is accessible to an attacker who gains access to the storage.

Recommendation

Ensure that sensitive information is always encrypted before being stored.

In general, decrypt sensitive information only at the point where it is necessary for it to be used in cleartext.

Be aware that external processes often store the standard out and standard error streams of the application, causing logged sensitive information to be stored as well.

Example

The following example code logs user credentials (in this case, their password) to standard out in plaintext:

require 'Logger'

class UserSession
  @@logger = Logger.new STDOUT

  def login(username, password)
    # ...
    @@logger.info "login with password: #{password})"
  end
end

Instead, the credentials should be masked or redacted before logging:

require 'Logger'

class UserSession
  @@logger = Logger.new STDOUT

  def login(username, password)
    # ...
    password_escaped = password.sub(/.*/, "[redacted]")
    @@logger.info "login with password: #{password_escaped})"
  end
end

References

M. Dowd, J. McDonald and J. Schuhm, The Art of Software Security Assessment, 1st Edition, Chapter 2 - 'Common Vulnerabilities of Encryption', p. 43. Addison Wesley, 2006.
M. Howard and D. LeBlanc, Writing Secure Code, 2nd Edition, Chapter 9 - 'Protecting Secret Data', p. 299. Microsoft, 2002.
Common Weakness Enumeration: CWE-312.
Common Weakness Enumeration: CWE-359.
Common Weakness Enumeration: CWE-532.

github-actions · 2022-02-10T18:37:39Z

QHelp previews:

ruby/ql/src/queries/security/cwe-312/CleartextLogging.qhelp

Clear-text logging of sensitive information

Sensitive information that is stored unencrypted is accessible to an attacker who gains access to the storage.

Recommendation

Ensure that sensitive information is always encrypted before being stored.

In general, decrypt sensitive information only at the point where it is necessary for it to be used in cleartext.

Be aware that external processes often store the standard out and standard error streams of the application, causing logged sensitive information to be stored as well.

Example

The following example code logs user credentials (in this case, their password) to standard out in plaintext:

require 'Logger'

class UserSession
  @@logger = Logger.new STDOUT

  def login(username, password)
    # ...
    @@logger.info "login with password: #{password})"
  end
end

Instead, the credentials should be masked or redacted before logging:

require 'Logger'

class UserSession
  @@logger = Logger.new STDOUT

  def login(username, password)
    # ...
    password_escaped = password.sub(/.*/, "[redacted]")
    @@logger.info "login with password: #{password_escaped})"
  end
end

References

M. Dowd, J. McDonald and J. Schuhm, The Art of Software Security Assessment, 1st Edition, Chapter 2 - 'Common Vulnerabilities of Encryption', p. 43. Addison Wesley, 2006.
M. Howard and D. LeBlanc, Writing Secure Code, 2nd Edition, Chapter 9 - 'Protecting Secret Data', p. 299. Microsoft, 2002.
Common Weakness Enumeration: CWE-312.
Common Weakness Enumeration: CWE-359.
Common Weakness Enumeration: CWE-532.

hmac

This looks great. I understand it better on the second read through!

I've left one comment about a possible simiplifcation, but otherwise 👍. I've also kicked off a DCA run that should be done by the time you read this.

hmac · 2022-02-17T03:39:12Z

ruby/ql/lib/codeql/ruby/security/CleartextLoggingCustomizations.qll

+        def.hasAdjacentReads(any(MaskingReplacerSanitizedNode read).asExpr(), this.asExpr())
+      )
+    }
+  }


I think this can be simplified by replacing it with something like this:

/** * Like `MaskingReplacerSanitizer` but updates the receiver. * Taint is thereby cleared for any subsequent read. */ private class InPlaceMaskingReplacerSanitizer extends Sanitizer { InPlaceMaskingReplacerSanitizer() { exists(MaskingReplacerSanitizer m | m.getMethodName() = ["gsub!", "sub!"] | m.getReceiver() = this ) } }

The idea being that we make the receiver of sub! be a sanitizer, which (I believe) removes it and any subsequent reads of the same variable from the dataflow graph.

Ah, that's really nice - much neater and truer to the intent of this sanitizer.

github-actions · 2022-02-17T13:13:00Z

QHelp previews:

ruby/ql/src/queries/security/cwe-312/CleartextLogging.qhelp

Clear-text logging of sensitive information

Sensitive information that is stored unencrypted is accessible to an attacker who gains access to the storage.

Recommendation

Ensure that sensitive information is always encrypted before being stored.

In general, decrypt sensitive information only at the point where it is necessary for it to be used in cleartext.

Be aware that external processes often store the standard out and standard error streams of the application, causing logged sensitive information to be stored as well.

Example

The following example code logs user credentials (in this case, their password) to standard out in plaintext:

require 'Logger'

class UserSession
  @@logger = Logger.new STDOUT

  def login(username, password)
    # ...
    @@logger.info "login with password: #{password})"
  end
end

Instead, the credentials should be masked or redacted before logging:

require 'Logger'

class UserSession
  @@logger = Logger.new STDOUT

  def login(username, password)
    # ...
    password_escaped = password.sub(/.*/, "[redacted]")
    @@logger.info "login with password: #{password_escaped})"
  end
end

References

M. Dowd, J. McDonald and J. Schuhm, The Art of Software Security Assessment, 1st Edition, Chapter 2 - 'Common Vulnerabilities of Encryption', p. 43. Addison Wesley, 2006.
M. Howard and D. LeBlanc, Writing Secure Code, 2nd Edition, Chapter 9 - 'Protecting Secret Data', p. 299. Microsoft, 2002.
Common Weakness Enumeration: CWE-312.
Common Weakness Enumeration: CWE-359.
Common Weakness Enumeration: CWE-532.

github-actions · 2022-02-17T15:33:50Z

QHelp previews:

ruby/ql/src/queries/security/cwe-312/CleartextLogging.qhelp

Clear-text logging of sensitive information

Sensitive information that is stored unencrypted is accessible to an attacker who gains access to the storage.

Recommendation

Ensure that sensitive information is always encrypted before being stored.

In general, decrypt sensitive information only at the point where it is necessary for it to be used in cleartext.

Be aware that external processes often store the standard out and standard error streams of the application, causing logged sensitive information to be stored as well.

Example

The following example code logs user credentials (in this case, their password) to standard out in plaintext:

require 'Logger'

class UserSession
  @@logger = Logger.new STDOUT

  def login(username, password)
    # ...
    @@logger.info "login with password: #{password})"
  end
end

Instead, the credentials should be masked or redacted before logging:

require 'Logger'

class UserSession
  @@logger = Logger.new STDOUT

  def login(username, password)
    # ...
    password_escaped = password.sub(/.*/, "[redacted]")
    @@logger.info "login with password: #{password_escaped})"
  end
end

References

M. Dowd, J. McDonald and J. Schuhm, The Art of Software Security Assessment, 1st Edition, Chapter 2 - 'Common Vulnerabilities of Encryption', p. 43. Addison Wesley, 2006.
M. Howard and D. LeBlanc, Writing Secure Code, 2nd Edition, Chapter 9 - 'Protecting Secret Data', p. 299. Microsoft, 2002.
Common Weakness Enumeration: CWE-312.
Common Weakness Enumeration: CWE-359.
Common Weakness Enumeration: CWE-532.

Changes made

alexrford added WIP This is a work-in-progress, do not merge yet! Ruby labels Jan 23, 2022

Ruby: add shared SensitiveDataHeuristics.qll

cfb2d7f

alexrford force-pushed the ruby/clear-text-logging branch from 8a28239 to 3b04619 Compare January 28, 2022 17:10

github-actions bot added the documentation label Jan 28, 2022

alexrford removed the WIP This is a work-in-progress, do not merge yet! label Jan 28, 2022

alexrford changed the title ~~Ruby: Add rb/clear-text-logging-sensitive-data query [WIP]~~ Ruby: Add rb/clear-text-logging-sensitive-data query Jan 28, 2022

alexrford added 6 commits January 28, 2022 17:24

Ruby: Implement rb/clear-text-logging-sensitive-data

91ccd30

Ruby: cleartext logging - remove an unnecessary abstract class

4fc9128

Ruby: cleartext logging test output

7ed4478

Ruby: Add CleartextLogging.qhelp

186623f

Ruby: QL format

7fec2d2

Ruby: rb/clear-text-logging-sensitive-data changenote

269722f

alexrford force-pushed the ruby/clear-text-logging branch from 14a4919 to 269722f Compare January 28, 2022 17:28

Ruby: missing QLDoc

57e958c

alexrford marked this pull request as ready for review January 28, 2022 18:43

alexrford requested a review from a team as a code owner January 28, 2022 18:43

hvitved previously requested changes Feb 3, 2022

View reviewed changes

ruby/ql/lib/codeql/ruby/security/CleartextLoggingCustomizations.qll Outdated Show resolved Hide resolved

hmac reviewed Feb 3, 2022

View reviewed changes

Ruby: comment and node description fixes

81ed5d0

Co-authored-by: Harry Maclean <[email protected]>

alexrford added 5 commits February 10, 2022 15:50

Ruby: rb/clear-text-logging-sensitive-data - match on CFG nodes rathe…

59ab384

…r than AST nodes

Ruby: track masked variables potentially containing sensitive data mo…

7b4af39

…re accurately

Ruby: drop SanitizerIn from ClearTextLoggingQuery

b46e4cc

Ruby: avoid marking mutator methods as being safe (i.e. not returning…

83a3808

… sensitive data)

Ruby: add a test case for cleartext logging that uses NonCleartextPas…

7c1bd9a

…swordFlow

Ruby: fewer mappings from dataflow nodes to ast nodes

bc53570

Ruby: fix an alert

d55ba25

alexrford requested a review from hmac February 16, 2022 10:24

hmac previously approved these changes Feb 17, 2022

View reviewed changes

Ruby: Simplify sub!/gsub! sanitizers for cleartext logging query

33f4fff

alexrford dismissed hmac’s stale review via 33f4fff February 17, 2022 13:11

Merge remote-tracking branch 'origin/main' into ruby/clear-text-logging

dd383f9

hmac approved these changes Feb 20, 2022

View reviewed changes

alexrford merged commit 746290d into main Feb 21, 2022

alexrford deleted the ruby/clear-text-logging branch February 21, 2022 10:12

Ruby: Add rb/clear-text-logging-sensitive-data query #7713

Ruby: Add rb/clear-text-logging-sensitive-data query #7713

Uh oh!

Conversation

alexrford commented Jan 23, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Jan 28, 2022

Clear-text logging of sensitive information

Recommendation

Example

References

Uh oh!

github-actions bot commented Jan 28, 2022

Clear-text logging of sensitive information

Recommendation

Example

References

Uh oh!

github-actions bot commented Jan 28, 2022

Clear-text logging of sensitive information

Recommendation

Example

References

Uh oh!

github-actions bot commented Jan 28, 2022

Clear-text logging of sensitive information

Recommendation

Example

References

Uh oh!

Uh oh!

hmac left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

github-actions bot commented Feb 8, 2022

Clear-text logging of sensitive information

Recommendation

Example

References

Uh oh!

github-actions bot commented Feb 10, 2022

Clear-text logging of sensitive information

Recommendation

Example

References

Uh oh!

github-actions bot commented Feb 10, 2022

Clear-text logging of sensitive information

Recommendation

Example

References

Uh oh!

hmac left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Feb 17, 2022

Ruby: Add `rb/clear-text-logging-sensitive-data` query #7713

Ruby: Add `rb/clear-text-logging-sensitive-data` query #7713

alexrford commented Jan 23, 2022 •

edited

Loading