fix(security):Path Traversal Bug

[bhaskarvilles]

Unsanitized input from CLI argument flows into io.ioutil.ReadFile, where it is used as a path. This may result in a Path Traversal vulnerability and allow an attacker to read arbitrary files.

Signed-off-by: Bhaskar Ram [email protected]

[denyeart]

@bhaskarvilles Thanks for the find but it seems the fix does not work. See the error during integration test checks.

[bhaskarvilles]

@bhaskarvilles Thanks for the find but it seems the fix does not work. See the error during integration test checks.

There will be some missing arguments.. let me check and fix the integration tests.

[C0rWin]

I am not sure the fix actually addresses the path traversal problem. As far as I understand the problem, it could be solved by input sanitation, and I am not sure I understand how dereferencing pointer could solve it.

Moreover, I do not think this is a real issue here since only peers can pass parameters to the build and detect, hence I do not see how this could be exploited here. Might be @ale-linux could take a look here?

[C0rWin]

@bhaskarvilles Thanks for the find but it seems the fix does not work. See the error during integration test checks.

There will be some missing arguments.. let me check and fix the integration tests.

The problem is not with the arguments, the code is not compilable, please take a look at tests output:

cmd/detect/main.go:56:37: invalid operation: cannot indirect metadataFile (variable of type string)

[bhaskarvilles]

@bhaskarvilles Thanks for the find but it seems the fix does not work. See the error during integration test checks.

There will be some missing arguments.. let me check and fix the integration tests.

The problem is not with the arguments, the code is not compilable, please take a look at tests output:

cmd/detect/main.go:56:37: invalid operation: cannot indirect metadataFile (variable of type string)

Can you explain in mode detail ??

[C0rWin]

@bhaskarvilles Thanks for the find but it seems the fix does not work. See the error during integration test checks.

There will be some missing arguments.. let me check and fix the integration tests.

The problem is not with the arguments, the code is not compilable, please take a look at tests output: cmd/detect/main.go:56:37: invalid operation: cannot indirect metadataFile (variable of type string)

Can you explain in mode detail ??

After you fix it's no longer possible to compile the code, moreover I do not see how dereferencing of the pointer (assume for a while this is indeed a pointer), solves aforementioned path traversal vulnerability?

[bhaskarvilles]

@bhaskarvilles Thanks for the find but it seems the fix does not work. See the error during integration test checks.

There will be some missing arguments.. let me check and fix the integration tests.

The problem is not with the arguments, the code is not compilable, please take a look at tests output: cmd/detect/main.go:56:37: invalid operation: cannot indirect metadataFile (variable of type string)

Can you explain in mode detail ??

After you fix it's no longer possible to compile the code, moreover I do not see how dereferencing of the pointer (assume for a while this is indeed a pointer), solves aforementioned path traversal vulnerability?

Yes, it solves the issue, let me research on the new compilation methods. But I'm sure that it will definitely solves the problem.

[denyeart]

@bhaskarvilles Assuming you get the code to compile, please add an explanation in the PR and commit message of how the fix prevents the issue.

[C0rWin]

Yes, it solves the issue, let me research on the new compilation methods. But I'm sure that it will definitely solves the problem.

I'm pretty confident that to solve path traversal, you have to implement input sanitation to ensure path traversal could not be exploited. Moreover, in this particular case, since the input to the program is provided by another process and no user can control it, I do not think there is a risk of running into path traversal anyway.

This being said, I think it would be a good practice if, beyond the fix, you will also provide a test to showcase the issue, and we can also make sure that your fix addresses it.

[bhaskarvilles]

Yes, it solves the issue, let me research on the new compilation methods. But I'm sure that it will definitely solves the problem.

I'm pretty confident that to solve path traversal, you have to implement input sanitation to ensure path traversal could not be exploited. Moreover, in this particular case, since the input to the program is provided by another process and no user can control it, I do not think there is a risk of running into path traversal anyway.

This being said, I think it would be a good practice if, beyond the fix, you will also provide a test to showcase the issue, and we can also make sure that your fix addresses it.

Can you update the issue that resolves this problem ?

[denyeart]

@bhaskarvilles We are asserting that there is no issue possible here.

If you can describe how somebody could make such an exploit, and how you intend to resolve it, we can continue here. If you don't provide a detailed explanation, then we'll close this PR.

[bhaskarvilles]

@denyeart Here is the data flow & detailed explanation of this bug.

Data flow :

9 steps in 1 file

ccaas_builder/cmd/build/main.go 9 steps ln 64:39 os.Args[1],os.Args[1]sourceDir - SOURCE ln :39 os.Args[1] - SOURCE ln :39 os.Args[1] ln :2 sourceDir ln 66:37 sourceDirfilepath.Join(sourceDir, "/connection.json")connectionSrcFile ln 102:27 connectionSrcFile ln 107:49 connectionSrcFileioutil.ReadFile(connectionSrcFile)

connectionSrcFile

ioutil.ReadFile(connectionSrcFile)

[C0rWin]

@bhaskarvilles, can you explain the fix, please? Why does the code change address the problem? I think it would be helpful if you could provide a test that manifests the exploit to show the fix addresses it.

[C0rWin]

@denyeart Here is the data flow & detailed explanation of this bug.

Data flow :

9 steps in 1 file

ccaas_builder/cmd/build/main.go 9 steps ln 64:39 os.Args[1],os.Args[1]sourceDir - SOURCE ln :39 os.Args[1] - SOURCE ln :39 os.Args[1] ln :2 sourceDir ln 66:37 sourceDirfilepath.Join(sourceDir, "/connection.json")connectionSrcFile ln 102:27 connectionSrcFile ln 107:49 connectionSrcFileioutil.ReadFile(connectionSrcFile)

connectionSrcFile

ioutil.ReadFile(connectionSrcFile)

Please note that parameters to os.Args[1] are provided by peer process and out of the user's control. Moreover, please note filepath.Join(sourceDir, "/connection.json") which makes the potential scope of path traversal very limited