From dd31be43e07d901bafb543c8f65be8e9b5abb364 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alvaro=20Mu=C3=B1oz?= Date: Fri, 3 Feb 2023 09:35:22 +0100 Subject: [PATCH] Support for Twirp framework --- ruby/ql/lib/change-notes/2023-02-03-twirp.md | 4 ++ ruby/ql/lib/codeql/ruby/Frameworks.qll | 1 + ruby/ql/lib/codeql/ruby/frameworks/Twirp.qll | 71 +++++++++++++++++++ .../library-tests/frameworks/Twirp/Gemfile | 5 ++ .../library-tests/frameworks/Twirp/Twirp.ql | 10 +++ .../Twirp/hello_world/service.proto | 15 ++++ .../Twirp/hello_world/service_pb.rb | 22 ++++++ .../Twirp/hello_world/service_twirp.rb | 17 +++++ .../frameworks/Twirp/hello_world_client.rb | 13 ++++ .../frameworks/Twirp/hello_world_server.rb | 29 ++++++++ .../frameworks/Twirp/tests.expected | 1 + 11 files changed, 188 insertions(+) create mode 100644 ruby/ql/lib/change-notes/2023-02-03-twirp.md create mode 100644 ruby/ql/lib/codeql/ruby/frameworks/Twirp.qll create mode 100644 ruby/ql/test/library-tests/frameworks/Twirp/Gemfile create mode 100644 ruby/ql/test/library-tests/frameworks/Twirp/Twirp.ql create mode 100644 ruby/ql/test/library-tests/frameworks/Twirp/hello_world/service.proto create mode 100644 ruby/ql/test/library-tests/frameworks/Twirp/hello_world/service_pb.rb create mode 100644 ruby/ql/test/library-tests/frameworks/Twirp/hello_world/service_twirp.rb create mode 100644 ruby/ql/test/library-tests/frameworks/Twirp/hello_world_client.rb create mode 100644 ruby/ql/test/library-tests/frameworks/Twirp/hello_world_server.rb create mode 100644 ruby/ql/test/library-tests/frameworks/Twirp/tests.expected diff --git a/ruby/ql/lib/change-notes/2023-02-03-twirp.md b/ruby/ql/lib/change-notes/2023-02-03-twirp.md new file mode 100644 index 00000000000..c6f0d48837e --- /dev/null +++ b/ruby/ql/lib/change-notes/2023-02-03-twirp.md @@ -0,0 +1,4 @@ +--- + category: minorAnalysis +--- +* Support for [Twirp framework](https://twitchtv.github.io/twirp/docs/intro.html). diff --git a/ruby/ql/lib/codeql/ruby/Frameworks.qll b/ruby/ql/lib/codeql/ruby/Frameworks.qll index c03b6427dec..735178388e8 100644 --- a/ruby/ql/lib/codeql/ruby/Frameworks.qll +++ b/ruby/ql/lib/codeql/ruby/Frameworks.qll @@ -27,3 +27,4 @@ private import codeql.ruby.frameworks.ActionDispatch private import codeql.ruby.frameworks.PosixSpawn private import codeql.ruby.frameworks.StringFormatters private import codeql.ruby.frameworks.Json +private import codeql.ruby.frameworks.Twirp diff --git a/ruby/ql/lib/codeql/ruby/frameworks/Twirp.qll b/ruby/ql/lib/codeql/ruby/frameworks/Twirp.qll new file mode 100644 index 00000000000..bd8c2cc3bc5 --- /dev/null +++ b/ruby/ql/lib/codeql/ruby/frameworks/Twirp.qll @@ -0,0 +1,71 @@ +/** + * Provides classes for modeling the `Twirp` framework. + */ + +private import codeql.ruby.DataFlow +private import codeql.ruby.CFG +private import codeql.ruby.ApiGraphs +private import codeql.ruby.AST as Ast +private import codeql.ruby.security.ServerSideRequestForgeryCustomizations +private import codeql.ruby.Concepts + +/** + * Provides classes for modeling the `Twirp` framework. + */ +module Twirp { + /** + * A Twirp service instantiation + */ + class ServiceInstantiation extends DataFlow::CallNode { + ServiceInstantiation() { + this = + API::getTopLevelMember("Twirp").getMember("Service").getASubclass*().getAnInstantiation() + } + + DataFlow::LocalSourceNode getHandlerSource() { result = this.getArgument(0).getALocalSource() } + + API::Node getHandlerClassApiNode() { result.getAnInstantiation() = this.getHandlerSource() } + + DataFlow::LocalSourceNode getHandlerClassDataFlowNode() { + result = this.getHandlerClassApiNode().asSource() + } + + Ast::Module getHandlerClassAstNode() { + result = + this.getHandlerClassDataFlowNode() + .asExpr() + .(CfgNodes::ExprNodes::ConstantReadAccessCfgNode) + .getExpr() + .getModule() + } + + Ast::Method getHandlerMethod() { result = this.getHandlerClassAstNode().getAnInstanceMethod() } + } + + /** + * A Twirp client + */ + class ClientInstantiation extends DataFlow::CallNode { + ClientInstantiation() { + this = + API::getTopLevelMember("Twirp").getMember("Client").getASubclass*().getAnInstantiation() + } + } + + /** The URL of a Twirp service, considered as a sink. */ + class ServiceUrlAsSsrfSink extends ServerSideRequestForgery::Sink { + ServiceUrlAsSsrfSink() { exists(ClientInstantiation c | c.getArgument(0) = this) } + } + + /** A parameter that will receive parts of the url when handling an incoming request. */ + class UnmarshaledParameter extends Http::Server::RequestInputAccess::Range, + DataFlow::ParameterNode { + UnmarshaledParameter() { + exists(ServiceInstantiation i | i.getHandlerMethod().getParameter(0) = this.asParameter()) + } + + override string getSourceType() { result = "Twirp Unmarhaled Parameter" } + + override Http::Server::RequestInputKind getKind() { result = Http::Server::bodyInputKind() } + } +} diff --git a/ruby/ql/test/library-tests/frameworks/Twirp/Gemfile b/ruby/ql/test/library-tests/frameworks/Twirp/Gemfile new file mode 100644 index 00000000000..9f49fa5ad78 --- /dev/null +++ b/ruby/ql/test/library-tests/frameworks/Twirp/Gemfile @@ -0,0 +1,5 @@ +source "https://rubygems.org" + +gem "rack" +gem "webrick" +gem "twirp" diff --git a/ruby/ql/test/library-tests/frameworks/Twirp/Twirp.ql b/ruby/ql/test/library-tests/frameworks/Twirp/Twirp.ql new file mode 100644 index 00000000000..e6b1c36bde4 --- /dev/null +++ b/ruby/ql/test/library-tests/frameworks/Twirp/Twirp.ql @@ -0,0 +1,10 @@ +private import codeql.ruby.frameworks.Twirp +private import codeql.ruby.DataFlow + +query predicate sourceTest(DataFlow::Node s) { s instanceof Twirp::UnmarshaledParameter } + +query predicate ssrfSinkTest(DataFlow::Node n) { n instanceof Twirp::ServiceUrlAsSsrfSink } + +query predicate serviceInstantiationTest(DataFlow::Node n) { + n instanceof Twirp::ServiceInstantiation +} diff --git a/ruby/ql/test/library-tests/frameworks/Twirp/hello_world/service.proto b/ruby/ql/test/library-tests/frameworks/Twirp/hello_world/service.proto new file mode 100644 index 00000000000..f7b4a818e2f --- /dev/null +++ b/ruby/ql/test/library-tests/frameworks/Twirp/hello_world/service.proto @@ -0,0 +1,15 @@ +syntax = "proto3"; +package example.hello_world; + + +service HelloWorld { + rpc Hello(HelloRequest) returns (HelloResponse); +} + +message HelloRequest { + string name = 1; +} + +message HelloResponse { + string message = 1; +} diff --git a/ruby/ql/test/library-tests/frameworks/Twirp/hello_world/service_pb.rb b/ruby/ql/test/library-tests/frameworks/Twirp/hello_world/service_pb.rb new file mode 100644 index 00000000000..10de1c1f7e8 --- /dev/null +++ b/ruby/ql/test/library-tests/frameworks/Twirp/hello_world/service_pb.rb @@ -0,0 +1,22 @@ +# Generated by the protocol buffer compiler. DO NOT EDIT! +# source: hello_world/service.proto + +require 'google/protobuf' + +Google::Protobuf::DescriptorPool.generated_pool.build do + add_file("hello_world/service.proto", :syntax => :proto3) do + add_message "example.hello_world.HelloRequest" do + optional :name, :string, 1 + end + add_message "example.hello_world.HelloResponse" do + optional :message, :string, 1 + end + end +end + +module Example + module HelloWorld + HelloRequest = ::Google::Protobuf::DescriptorPool.generated_pool.lookup("example.hello_world.HelloRequest").msgclass + HelloResponse = ::Google::Protobuf::DescriptorPool.generated_pool.lookup("example.hello_world.HelloResponse").msgclass + end +end diff --git a/ruby/ql/test/library-tests/frameworks/Twirp/hello_world/service_twirp.rb b/ruby/ql/test/library-tests/frameworks/Twirp/hello_world/service_twirp.rb new file mode 100644 index 00000000000..ff3b9cc4a81 --- /dev/null +++ b/ruby/ql/test/library-tests/frameworks/Twirp/hello_world/service_twirp.rb @@ -0,0 +1,17 @@ +# Code generated by protoc-gen-twirp_ruby 1.10.0, DO NOT EDIT. +require 'twirp' +require_relative 'service_pb.rb' + +module Example + module HelloWorld + class HelloWorldService < ::Twirp::Service + package 'example.hello_world' + service 'HelloWorld' + rpc :Hello, HelloRequest, HelloResponse, :ruby_method => :hello + end + + class HelloWorldClient < ::Twirp::Client + client_for HelloWorldService + end + end +end diff --git a/ruby/ql/test/library-tests/frameworks/Twirp/hello_world_client.rb b/ruby/ql/test/library-tests/frameworks/Twirp/hello_world_client.rb new file mode 100644 index 00000000000..68c63796a1e --- /dev/null +++ b/ruby/ql/test/library-tests/frameworks/Twirp/hello_world_client.rb @@ -0,0 +1,13 @@ +require 'rack' + +require_relative 'hello_world/service_twirp.rb' + +# test: ssrfSink +c = Example::HelloWorld::HelloWorldClient.new("http://localhost:8080/twirp") + +resp = c.hello(name: "World") +if resp.error + puts resp.error +else + puts resp.data.message +end diff --git a/ruby/ql/test/library-tests/frameworks/Twirp/hello_world_server.rb b/ruby/ql/test/library-tests/frameworks/Twirp/hello_world_server.rb new file mode 100644 index 00000000000..1aa0b9aa8de --- /dev/null +++ b/ruby/ql/test/library-tests/frameworks/Twirp/hello_world_server.rb @@ -0,0 +1,29 @@ +require 'rack' +require 'webrick' + +require_relative 'hello_world/service_twirp.rb' + +class HelloWorldHandler + # test: request + def hello(req, env) + puts ">> Hello #{req.name}" + {message: "Hello #{req.name}"} + end +end + +class FakeHelloWorldHandler + # test: !request + def hello(req, env) + puts ">> Hello #{req.name}" + {message: "Hello #{req.name}"} + end +end + +handler = HelloWorldHandler.new() +# test: serviceInstantiation +service = Example::HelloWorld::HelloWorldService.new(handler) + +path_prefix = "/twirp/" + service.full_name +server = WEBrick::HTTPServer.new(Port: 8080) +server.mount path_prefix, Rack::Handler::WEBrick, service +server.start diff --git a/ruby/ql/test/library-tests/frameworks/Twirp/tests.expected b/ruby/ql/test/library-tests/frameworks/Twirp/tests.expected new file mode 100644 index 00000000000..429c96f804a --- /dev/null +++ b/ruby/ql/test/library-tests/frameworks/Twirp/tests.expected @@ -0,0 +1 @@ +The query depends on an extensional predicate sinkModel which has not been defined.