From b66ea6ed7218fc97a8f43865acf092beb29e9976 Mon Sep 17 00:00:00 2001 From: Harry Maclean Date: Fri, 25 Nov 2022 17:21:51 +1300 Subject: [PATCH] Ruby: Simplify ActionMailbox modeling --- .../codeql/ruby/frameworks/ActionMailbox.qll | 43 +++++++------------ .../action_mailbox/ActionMailbox.expected | 2 +- .../action_mailbox/ActionMailbox.ql | 5 ++- 3 files changed, 19 insertions(+), 31 deletions(-) diff --git a/ruby/ql/lib/codeql/ruby/frameworks/ActionMailbox.qll b/ruby/ql/lib/codeql/ruby/frameworks/ActionMailbox.qll index 8fb0def0a33..5416fca62a4 100644 --- a/ruby/ql/lib/codeql/ruby/frameworks/ActionMailbox.qll +++ b/ruby/ql/lib/codeql/ruby/frameworks/ActionMailbox.qll @@ -30,8 +30,8 @@ module ActionMailbox { * `ActionMailbox::Base#mail`, which is equivalent. The returned object * contains data from the incoming email. */ - class MailCall extends DataFlow::CallNode, Mail::Message::Range { - MailCall() { + class Mail extends DataFlow::CallNode { + Mail() { this = [ controller().getAnInstanceSelf().getAMethodCall("inbound_email").getAMethodCall("mail"), @@ -41,34 +41,21 @@ module ActionMailbox { } /** - * Models classes from the `mail` library. - * Version: 2.7.1. + * A method call on a `Mail::Message` object which may return data from a remote source. */ - module Mail { - /** - * An instance of `Mail::Message`. - */ - class Message extends DataFlow::Node instanceof Message::Range { } - - module Message { - abstract class Range extends DataFlow::Node { } + private class RemoteContent extends DataFlow::CallNode, RemoteFlowSource::Range { + RemoteContent() { + this = + any(Mail m) + .(DataFlow::LocalSourceNode) + .getAMethodCall([ + "body", "to", "from", "raw_source", "subject", "from_address", + "recipients_addresses", "cc_addresses", "bcc_addresses", "in_reply_to", + "references", "reply_to", "raw_envelope", "to_s", "encoded", "header", "bcc", "cc", + "text_part", "html_part" + ]) } - /** - * A method call on a `Mail::Message` object which may return data from a remote source. - */ - class RemoteContent extends DataFlow::CallNode, RemoteFlowSource::Range { - RemoteContent() { - this.getReceiver() instanceof Message and - this.getMethodName() = - [ - "body", "to", "from", "raw_source", "subject", "from_address", "recipients_addresses", - "cc_addresses", "bcc_addresses", "in_reply_to", "references", "reply_to", - "raw_envelope", "to_s", "encoded", "header", "bcc", "cc", "text_part", "html_part" - ] - } - - override string getSourceType() { result = "ActionMailbox" } - } + override string getSourceType() { result = "ActionMailbox" } } } diff --git a/ruby/ql/test/library-tests/frameworks/action_mailbox/ActionMailbox.expected b/ruby/ql/test/library-tests/frameworks/action_mailbox/ActionMailbox.expected index 6cce834e488..f0325269fb0 100644 --- a/ruby/ql/test/library-tests/frameworks/action_mailbox/ActionMailbox.expected +++ b/ruby/ql/test/library-tests/frameworks/action_mailbox/ActionMailbox.expected @@ -7,7 +7,7 @@ messageInstances | action_mailbox.rb:6:5:6:10 | call to mail | | action_mailbox.rb:10:5:10:8 | call to mail | | action_mailbox.rb:16:9:16:12 | call to mail | -remoteContent +remoteFlowSources | action_mailbox.rb:3:5:3:13 | call to body | | action_mailbox.rb:4:5:4:11 | call to to | | action_mailbox.rb:6:5:6:13 | call to to | diff --git a/ruby/ql/test/library-tests/frameworks/action_mailbox/ActionMailbox.ql b/ruby/ql/test/library-tests/frameworks/action_mailbox/ActionMailbox.ql index 2477678b78f..e0a529dc6c6 100644 --- a/ruby/ql/test/library-tests/frameworks/action_mailbox/ActionMailbox.ql +++ b/ruby/ql/test/library-tests/frameworks/action_mailbox/ActionMailbox.ql @@ -1,8 +1,9 @@ private import codeql.ruby.frameworks.ActionMailbox private import codeql.ruby.DataFlow +private import codeql.ruby.dataflow.RemoteFlowSources query predicate processMethods(ActionMailbox::Process p) { any() } -query predicate messageInstances(ActionMailbox::Mail::Message c) { any() } +query predicate messageInstances(ActionMailbox::Mail c) { any() } -query predicate remoteContent(ActionMailbox::Mail::RemoteContent r) { any() } +query predicate remoteFlowSources(RemoteFlowSource r) { any() }