Skip to content

Commit 2e2fcd4

Browse files
committed
Ruby: Consider Object#inspect a log sanitizer
The behaviour of `Object#inspect` depends on whether it has been overridden by a subclass, but it will typically produce output on a single line. Calling `inspect` on a String will replace newlines with `\n`, which is then safe for interpolation into a log line.
1 parent 762ebad commit 2e2fcd4

File tree

3 files changed

+21
-2
lines changed

3 files changed

+21
-2
lines changed

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

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,14 @@ class StringReplaceSanitizer extends Sanitizer {
6060
}
6161
}
6262

63+
/**
64+
* A call to `Object#inspect`, considered as a sanitizer.
65+
* This is because `inspect` will replace newlines in strings with `\n`.
66+
*/
67+
class InspectSanitizer extends Sanitizer {
68+
InspectSanitizer() { this.(DataFlow::CallNode).getMethodName() = "inspect" }
69+
}
70+
6371
/**
6472
* A call to an HTML escape method is considered to sanitize its input.
6573
*/

ruby/ql/test/query-tests/security/cwe-117/LogInjection.expected

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ edges
1111
| app/controllers/users_controller.rb:33:5:33:31 | ... = ... : | app/controllers/users_controller.rb:35:33:35:55 | ... + ... |
1212
| app/controllers/users_controller.rb:33:19:33:25 | call to cookies : | app/controllers/users_controller.rb:33:19:33:31 | ...[...] : |
1313
| app/controllers/users_controller.rb:33:19:33:31 | ...[...] : | app/controllers/users_controller.rb:33:5:33:31 | ... = ... : |
14+
| app/controllers/users_controller.rb:49:19:49:24 | call to params : | app/controllers/users_controller.rb:49:19:49:30 | ...[...] |
1415
nodes
1516
| app/controllers/users_controller.rb:15:19:15:24 | call to params : | semmle.label | call to params : |
1617
| app/controllers/users_controller.rb:15:19:15:30 | ...[...] : | semmle.label | ...[...] : |
@@ -26,6 +27,8 @@ nodes
2627
| app/controllers/users_controller.rb:33:19:33:31 | ...[...] : | semmle.label | ...[...] : |
2728
| app/controllers/users_controller.rb:34:33:34:43 | unsanitized | semmle.label | unsanitized |
2829
| app/controllers/users_controller.rb:35:33:35:55 | ... + ... | semmle.label | ... + ... |
30+
| app/controllers/users_controller.rb:49:19:49:24 | call to params : | semmle.label | call to params : |
31+
| app/controllers/users_controller.rb:49:19:49:30 | ...[...] | semmle.label | ...[...] |
2932
subpaths
3033
#select
3134
| app/controllers/users_controller.rb:16:19:16:29 | unsanitized | app/controllers/users_controller.rb:15:19:15:24 | call to params : | app/controllers/users_controller.rb:16:19:16:29 | unsanitized | Log entry depends on a $@. | app/controllers/users_controller.rb:15:19:15:24 | call to params | user-provided value |
@@ -34,3 +37,4 @@ subpaths
3437
| app/controllers/users_controller.rb:27:16:27:39 | ... + ... | app/controllers/users_controller.rb:15:19:15:24 | call to params : | app/controllers/users_controller.rb:27:16:27:39 | ... + ... | Log entry depends on a $@. | app/controllers/users_controller.rb:15:19:15:24 | call to params | user-provided value |
3538
| app/controllers/users_controller.rb:34:33:34:43 | unsanitized | app/controllers/users_controller.rb:33:19:33:25 | call to cookies : | app/controllers/users_controller.rb:34:33:34:43 | unsanitized | Log entry depends on a $@. | app/controllers/users_controller.rb:33:19:33:25 | call to cookies | user-provided value |
3639
| app/controllers/users_controller.rb:35:33:35:55 | ... + ... | app/controllers/users_controller.rb:33:19:33:25 | call to cookies : | app/controllers/users_controller.rb:35:33:35:55 | ... + ... | Log entry depends on a $@. | app/controllers/users_controller.rb:33:19:33:25 | call to cookies | user-provided value |
40+
| app/controllers/users_controller.rb:49:19:49:30 | ...[...] | app/controllers/users_controller.rb:49:19:49:24 | call to params : | app/controllers/users_controller.rb:49:19:49:30 | ...[...] | Log entry depends on a $@. | app/controllers/users_controller.rb:49:19:49:24 | call to params | user-provided value |

ruby/ql/test/query-tests/security/cwe-117/app/controllers/users_controller.rb

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,14 @@ def html_sanitization
3939
init_logger
4040

4141
sanitized = html_escape params[:baz]
42-
@logger.debug unsanitized # GOOD: sanitized user input
43-
@logger.debug "input: " + unsanitized # GOOD: sanitized user input
42+
@logger.debug sanitized # GOOD: sanitized user input
43+
@logger.debug "input: " + sanitized # GOOD: sanitized user input
44+
end
45+
46+
def inspect_sanitization
47+
init_logger
48+
49+
@logger.debug params[:foo] # BAD: unsanitized user input
50+
@logger.debug params[:foo].inspect # GOOD: sanitized user input
4451
end
4552
end

0 commit comments

Comments
 (0)