Path traversal issue with Plug.Upload

[wingyplus]

I have a function that receives %Plug.Upload{} struct as an argument and read a file inside that function:

def do_read_file(%Plug.Upload{path: path}) do
  # do verify file extension and content-type before reading a file.
  {:ok, content} = File.read(path)
  # processing content...
end

When I run sobelow, I found an interesting issue:

Traversal.FileModule: Directory Traversal in `File.read` - Low Confidence
File: redacted.ex
Line: 662
Function: do_read_file:656
Variable: path

My question is how can fix the issue that Traversal.FileModule suggest?

[nwai90]

I can't find a good way to fix this as well so that Sobelow will not flag it. Passing the path into a function that sanitizes it still results in Sobelow flagging an issue.

[wingyplus]

Do we need to document to the FileModule that this case maybe cause false negative?

[apoorv-2204]

I am facing the same issue, I am getting path through a function call.

[houllette]

Similar issue in newer raised issue, linking here to keep track of it and perform some issue clean-up.

[houllette]

So I realize that is is super delayed from the original comments, but I wanted to leave some thoughts that may be helpful!

Sobelow as a scanning tool is rather simplistic - all it knows currently is what known bad patterns look like in Elixir code; sometimes the bad patterns are vulnerable function calls and sometimes it's the lack of of an optional argument.

In the case of Traversal.FileModule findings, Sobelow simply triggers off the detection of any File function being invoked regardless of what's being passed into it (i.e. the path being provided could already be sanitized but Sobelow doesn't know that).

This is why in the output initially shared in this issue, Sobelow flagged it as "Low Confidence" - since it isn't confident it's an actual issue or not and it should be investigated further. This is why the False Positive functionality exists in Sobelow, to mark functions that Sobelow is detecting as being vulnerable as being ok to ignore.

Ideally in future iterations of Sobelow we can introduce Taint Analysis to detect whether user input is making it's way into sensitive functions and/or add the ability to detect when common sanitization techniques are applied to inputs to make them safe - but for now, applying sobelow_skip is the best path forward for this issue.

[apoorv-2204]

So I realize that is is super delayed from the original comments, but I wanted to leave some thoughts that may be helpful!

Sobelow as a scanning tool is rather simplistic - all it knows currently is what known bad patterns look like in Elixir code; sometimes the bad patterns are vulnerable function calls and sometimes it's the lack of of an optional argument.

In the case of Traversal.FileModule findings, Sobelow simply triggers off the detection of any File function being invoked regardless of what's being passed into it (i.e. the path being provided could already be sanitized but Sobelow doesn't know that).

This is why in the output initially shared in this issue, Sobelow flagged it as "Low Confidence" - since it isn't confident it's an actual issue or not and it should be investigated further. This is why the False Positive functionality exists in Sobelow, to mark functions that Sobelow is detecting as being vulnerable as being ok to ignore.

Ideally in future iterations of Sobelow we can introduce Taint Analysis to detect whether user input is making it's way into sensitive functions and/or add the ability to detect when common sanitization techniques are applied to inputs to make them safe - but for now, applying sobelow_skip is the best path forward for this issue.

thanks