google / google/rust-p9

Tests aren't even passing for your (formerly exploitable) dot-dot vulnerability fix.

Open
#15 1 comment 0 reactions 0 assignees View on GitHub
Dominant language
Rust
Stars
52
Forks
11
PR merge metrics
No merged PRs in 30d

Description

```
 user   main  /  tmp  rust-p9  cargo test
warning: field `ino` is never read
--> src/server/read_dir.rs:13:9
|
12 | pub struct DirEntry {
| -------- field in this struct
13 | pub ino: libc::ino64_t,
| ^^^
|
= note: `#[warn(dead_code)]` (part of `#[warn(unused)]`) on by default

warning: hiding a lifetime that's elided elsewhere is confusing
--> src/server/tests.rs:389:20
|
389 | fn readdir(server: &mut Server, fid: u32) -> Readdir {
| ^^^^^^^^^^^ ^^^^^^^ the same lifetime is hidden here
| |
| the lifetime is elided here
|
= help: the same lifetime is referred to in inconsistent ways, making the signature confusing
= note: `#[warn(mismatched_lifetime_syntaxes)]` on by default
help: use `'_` for type paths
|
389 | fn readdir(server: &mut Server, fid: u32) -> Readdir<'_> {
| ++++

warning: `p9` (lib) generated 1 warning
warning: `p9` (lib test) generated 2 warnings (1 duplicate) (run `cargo fix --lib -p p9 --tests` to apply 1 suggestion)
Finished `test` profile [unoptimized + debuginfo] target(s) in 0.05s
Running unittests src/lib.rs (target/debug/deps/p9-f8757bce710edb6f)

running 82 tests
test protocol::wire_format::test::data_decode ... ok
test protocol::wire_format::test::data_encode ... ok
test protocol::wire_format::test::integer_byte_size ... ok
test protocol::wire_format::test::integer_decode ... ok
test protocol::wire_format::test::integer_encode ... ok
test protocol::wire_format::test::invalid_string_decode ... ok
test protocol::wire_format::test::nested_decode ... ok
test protocol::wire_format::test::nested_encode ... ok
test protocol::wire_format::test::string_byte_size ... ok
test protocol::wire_format::test::string_decode ... ok
test protocol::wire_format::test::string_encode ... ok
test protocol::wire_format::test::struct_decode ... ok
test protocol::wire_format::test::struct_encode ... ok
test protocol::wire_format::test::vector_decode ... ok
test protocol::wire_format::test::vector_encode ... ok
test protocol::wire_format::test::zero_length_string ... ok
test server::read_dir::test::no_nul_byte - should panic ... ok
test server::read_dir::test::padded_cstrings ... ok
test server::tests::append_read_only_file_create ... ok
test protocol::wire_format::test::error_cases ... ok
test server::tests::append_read_write_file_create ... ok
test server::tests::append_trunc_read_only_file_open ... ok
test server::tests::append_read_only_file_open ... ok
test server::tests::append_read_write_file_open ... ok
test server::tests::append_trunc_read_write_file_open ... ok
test server::tests::append_trunc_wronly_file_open ... ok
test server::tests::append_write_only_file_open ... ok
test server::tests::clunk ... ok
test server::tests::create_append_trunc_read_only_file_open ... ok
test server::tests::create_append_read_write_file_open ... ok
test server::tests::create_append_read_only_file_open ... ok
test server::tests::append_wronly_file_create ... ok
test server::tests::create_append_trunc_read_write_file_open ... ok
test server::tests::create_append_trunc_wronly_file_open ... ok
test server::tests::create_append_wronly_file_open ... ok
test server::tests::create_excl_read_only_file_open ... ok
test server::tests::create_excl_read_write_file_open ... ok
test server::tests::create_existing_file ... ok
test server::tests::create_read_only_file_open ... ok
test server::tests::create_read_write_file_open ... ok
test server::tests::create_excl_wronly_file_open ... ok
test server::tests::create_trunc_read_only_file_open ... ok
test server::tests::create_trunc_read_write_file_open ... ok
test server::tests::create_trunc_wronly_file_open ... ok
test server::tests::create_write_only_file_open ... ok
test server::tests::get_attr ... ok
test server::tests::getlock_rdlck ... ok
test server::tests::getlock_rdlck_nolock ... ok
test server::tests::getlock_wrlck ... ok
test server::tests::invalid_joins ... ok
test server::tests::lcreate_set_len ... ok
test server::tests::lock_rdlck ... ok
test server::tests::lock_rdlck_no_open_file ... ok
test server::tests::lock_unlck ... ok
test server::tests::lock_unlck_no_lock ... ok
test server::tests::lock_wrlck ... ok
test server::tests::lock_unlck_relock ... ok
test server::tests::lock_wrlck_no_open_file ... ok
test server::tests::mkdir ... ok
test server::tests::path_joins ... ok
test server::tests::path_traversal_protection ... FAILED
test server::tests::read_only_file_create ... ok
test server::tests::read_only_file_open ... ok
test server::tests::read_write_file_create ... ok
test server::tests::readlink ... ok
test server::tests::read_write_file_open ... ok
test server::tests::set_dir_atime ... ok
test server::tests::rename_at ... ok
test server::tests::set_dir_mode ... ok
test server::tests::set_dir_mtime ... ok
test server::tests::set_file_atime ... ok
test server::tests::set_file_mode ... ok
test server::tests::set_file_mtime ... ok
test server::tests::set_len ... ok
test server::tests::trunc_read_only_file_open ... ok
test server::tests::tree_walk ... ok
test server::tests::trunc_write_only_file_open ... ok
test server::tests::trunc_read_write_file_open ... ok
test server::tests::unlink_all ... ok
test server::tests::write_only_file_open ... ok
test server::tests::write_only_file_create ... ok
test server::tests::huge_directory ... ok

failures:

---- server::tests::path_traversal_protection stdout ----

thread 'server::tests::path_traversal_protection' (295919) panicked at src/server/tests.rs:1368:36:
failed to walk .. from root: Os { code: 22, kind: InvalidInput, message: "Invalid argument" }

failures:
server::tests::path_traversal_protection

test result: FAILED. 81 passed; 1 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.18s

error: test failed, to rerun pass `--lib`
```

Come on, please, fix this security issue already.

Oh, and also make sure to fix the next issue, in line 1383 of src/server/tests.rs, where the fix which is supposed to catch uses of `..` doesn't catch `../../subdir` problems.

EDIT: I just wet in to check the Actions tab. No tests executed. Hm.

Contributor guide

Open the contributing guide

Research direction

Run cargo test and start with server::tests::path_traversal_protection in src/server/tests.rs, especially the failure at line 1368 and the additional case at line 1383. Trace the path-handling entry point exercised by these tests; done means cargo test passes and both .. and ../../subdir traversal attempts are rejected.

Written by the indexing model from the issue text.

Assessment

Tech stack
rust
Domain
backend, security
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Active
Clarity
Clearly specified
Newbie friendliness
68/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.