Preventing path traversal by plugins?

Hi! sqlc looks neat and I'm playing around with it.

The docs make this statement:

WASM plugins are fully sandboxed; they do not have access to the network, filesystem, or environment variables.

This caused me to look into how filesystem writes are performed. Am I misinterpreting the code, or does the use of path.Join allow for path traversal attacks? e.g. if a plugin emits a path beginning with ../../../. I don't speak any Go, but my read of the Clean function (which appears to be applied automatically to the result of Join) is that it only normalizes .. inside a path and preserves it at the front.

Apologies in advance if I've missed the defense against this.

Excellent question! As the documentation states, WASM plugins do not have any filesystem access. The output of a plugin is specified in the out parameter. I need to investigate if plugin output can escape that directory using "..".

@cbiffle

@cbiffle and I were chatting about this before he filed the issue, and I was able to confirm they can (as he was writing it up!). I applied this diff to the Rust plugin demo:

--- a/src/main.rs
+++ b/src/main.rs
@@ -21,7 +21,7 @@ pub fn serialize_codegen_response(resp: &plugin::CodeGenResponse) -> Vec<u8> {

 pub fn create_codegen_response() -> plugin::CodeGenResponse {
     let mut file = plugin::File::default();
-    file.name = "hello.txt".to_string();
+    file.name = "../../../../../../../../../tmp/escaped.txt".to_string();
     file.contents = "Hello World".as_bytes().to_vec();

     let mut resp = plugin::CodeGenResponse::default();

and after running it:

% cat /tmp/escaped.txt
Hello World

WASM plugins do not have any filesystem access

It appears that they don't have filesystem access in the WASI sense, but that's a narrower statement. FWIW I also can't work out how to read the filesystem from a plugin, only write it, so, that's good.

@jgallagher

@jgallagher that looks conclusive to me. I'll get a PR up to fix the behavior. This could be a breaking change, so I'll need to decide if it can write to locations next to and under sqlc.json or the specified out directory.

Are you using sqlc at Oxide?

Assuming this gets fixed, I might suggest rephrasing that statement to "plugins can only write files into their output directory," since "no access to the filesystem" suggests to me that they couldn't, say, deposit malware to it or fill it up -- and to be useful plugins the writing part is important!

Are you using sqlc at Oxide?

Not currently, but John and I both think it's a really neat idea.

Yeah, it's a bit confusing to say that the plugins can write files, as they can't due to the sandbox. sqlc takes the output from the plugin and uses that to write the output files. I'm sure we can figure out some language that makes it clear what's happening.

nod Something like "plugins can send commands to sqlc to have files created in the output directory, but can't directly access the filesystem"

This doesn't include the necessary tests yet, but I believe #3195 should fix the issue.

Alright, this should be fixed on main. Can you build sqlc from main and confirm your plugin now generates an error?

Confirmed:

% sqlc-dev generate
# package sqlc-gen-rust
error generating code: invalid file output path: /tmp/escaped.txt

Thanks!

Navigation Menu

Repository navigation

Preventing path traversal by plugins? #3194

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Preventing path traversal by plugins? #3194

Description

Activity

kyleconroy commented on Feb 12, 2024

jgallagher commented on Feb 12, 2024

cbiffle commented on Feb 12, 2024

kyleconroy commented on Feb 12, 2024

cbiffle commented on Feb 12, 2024

kyleconroy commented on Feb 12, 2024

cbiffle commented on Feb 12, 2024

kyleconroy commented on Feb 12, 2024

kyleconroy commented on Feb 14, 2024

jgallagher commented on Feb 15, 2024

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions