Ruby: Taint does not flow into do-end blocks

[minusworld]

Describe the bug
This taint rule does not appear to flow into the do-end block. I want it to match the # ruleid inside the block.
The block looks like this:

  # ruleid: avoid-tainted-http-request
  Net::HTTP.start(uri.host, uri.port) do |http|
    # ruleid: avoid-tainted-http-request
    req = Net::HTTP::Get.new uri
    resp = http.request request
  end

To Reproduce
https://semgrep.dev/s/GwQe

What is the priority of the bug to you?

• P0: blocking your adoption of Semgrep or workflow
• P1: important to fix or quite annoying
• P2: regular bug that should get fixed

[emjin]

cc @IagoAbal @mmcqd

[IagoAbal]

Do you need to match both the block and the Get inside the block with the exact same rule? Most likely this is the result of taint-mode removing those kind of "duplicate matches", see PR #3781. If taint finds two matches, and one is strictly contained in the other, it will only report the larger one.

[IagoAbal]

I think this is mainly an issue of the Ruby front-end that encodes:

Net::HTTP.start(uri.host, uri.port) do |http|
    # ruleid: avoid-tainted-http-request
    req = Net::HTTP::Get.new uri
    resp = http.request request
end

in Generic as

  Net::HTTP.start(uri.host, uri.port, |http|
    # ruleid: avoid-tainted-http-request
    req = Net::HTTP::Get.new uri
    resp = http.request request)

which I don't find very accurate. And that encoding causes that matching the call to Net::HTTP.start also matches the entire do-end block.

[r2c-demo]

This issue is synced in Linear at https://linear.app/r2c/issue/PA-360/ruby-taint-does-not-flow-into-do-end-blocks.

[IagoAbal]

And that encoding causes that matching the call to Net::HTTP.start also matches the entire do-end block.

Well, I didn't know about Ruby blocks and I thought they had different semantics.. sorry if confused things. So it seems that the do-end block is a lambda that is passed as an argument to the method that precedes it?

Anyways, perhaps we could instead encode:

Net::HTTP.start(uri.host, uri.port) do |http|
    # ruleid: avoid-tainted-http-request
    req = Net::HTTP::Get.new uri
    resp = http.request request
end

this other way:

 Net::HTTP.start(uri.host, uri.port)({|http|
    req = Net::HTTP::Get.new uri
    resp = http.request request})

So that Net::HTTP.start(...) will not match the entire do-end block as it does now (see https://semgrep.dev/s/lZOo), which may not be entirely intuitive. Then you should get the behavior that you expected.

[IagoAbal]

This also requires taint analysis to handle nested functions.