Ruby: add rb/clear-text-storage-sensitive-data query

[alexrford]

This is #8306 again, which I'm unable to retarget against main and reopen.

The majority of this query is identical to rb/clear-text-logging-sensitive-data, with the main differences being in the sinks (file writes and other persistent writes rather than logging calls), and in reporting alerts at the sink rather than the source to make it easier to classify FPs with flow from a test file. The latter change may also be useful for the logging version of this query, but it appears to be a more common case with this query.

As this changes the shared SensitiveDataHeuristics.qll library, it may have a minor impact on results for JS/Python queries relying on the notSensitiveRegexp/0 predicate. See the comments at #8306 (comment) for some context on this change.

[github-actions]

QHelp previews:

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

Clear-text storage 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 stores user credentials (in this case, their password) to disk in plain text:

class UserSession
  def login(username, password)
    # ...
    logfile = File.open("login_attempts.log")
    logfile.puts "login with password: #{password})"
  end
end

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

class UserSession
  def login(username, password)
    # ...
    password_escaped = password.sub(/.*/, "[redacted]")
    logfile = File.open("login_attempts.log")
    logfile.puts "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]

QHelp previews:

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

Clear-text storage 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 stores user credentials (in this case, their password) to disk in plain text:

class UserSession
  def login(username, password)
    # ...
    logfile = File.open("login_attempts.log")
    logfile.puts "login with password: #{password})"
  end
end

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

class UserSession
  def login(username, password)
    # ...
    password_escaped = password.sub(/.*/, "[redacted]")
    logfile = File.open("login_attempts.log")
    logfile.puts "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.

[yoff]

As this changes the shared SensitiveDataHeuristics.qll library, it may have a minor impact on results for JS/Python queries relying on the notSensitiveRegexp/0 predicate. See the comments at #8306 (comment) for some context on this change.

I guess a suitable DCA run could give some visibility into potential changes. Otherwise Python looks fine to me. @RasmusWL has worked on sensitive data and may have an opinion.

[erik-krogh]

The only JS/Python change seems to be adding "random" to notSensitiveRegexp(), which seems fine to me.

JS 👍

[alexrford]

I guess a suitable DCA run could give some visibility into potential changes. Otherwise Python looks fine to me. @RasmusWL has worked on sensitive data and may have an opinion.

We lose one result in test code for py/clear-text-storage-sensitive-data in the standard code scanning set. It's not the kind of FP that this change is directly intended to avoid, where this random data represents some hash value. However, the result is low value, being in test code, and doesn't look to me like it would be a common pattern for production code.

[RasmusWL]

👍 for Python

I guess a suitable DCA run could give some visibility into potential changes. Otherwise Python looks fine to me. @RasmusWL has worked on sensitive data and may have an opinion.

We lose one result in test code for py/clear-text-storage-sensitive-data in the standard code scanning set. It's not the kind of FP that this change is directly intended to avoid, where this random data represents some hash value. However, the result is low value, being in test code, and doesn't look to me like it would be a common pattern for production code.

Agreed, that result was not interesting 👍