Fix buffer overflow in File::path()

[cbs228]

In netcdf-c, nc_inq_path() uses strlen() to calculate the length of the string. strlen() does not include the trailing NUL in its calculations… but the strcpy() does, and it includes one.

This makes the buffer allocated in File::path() always one byte too small to receive the path string. The result is a buffer overflow and undefined behavior. The presence of memory safety issues can be confirmed with valgrind.

Valgrind output

```txt # Note: look for "Invalid write of size 1" $ valgrind target/debug/deps/lib-28bd4a3f5ac29440 --test-threads=1 ==65245== Memcheck, a memory error detector ==65245== Copyright (C) 2002-2017, and GNU GPL'd, by Julian Seward et al. ==65245== Using Valgrind-3.15.0 and LibVEX; rerun with -h for copyright info ==65245== Command: target/debug/deps/lib-28bd4a3f5ac29440 --test-threads=1 ==65245== ==65245== Syscall param statx(file_name) points to unaddressable byte(s) ==65245== at 0x4B0270D: syscall (syscall.S:38) ==65245== by 0x2ACFEB: statx (weak.rs:94) ==65245== by 0x2ACFEB: std::sys::unix::fs::try_statx (fs.rs:132) ==65245== by 0x2A7E0E: std::sys::unix::fs::stat (fs.rs:995) ==65245== by 0x285934: metadata<&std::path::PathBuf> (fs.rs:1553) ==65245== by 0x285934: term::terminfo::searcher::get_dbpath_for_term (searcher.rs:50) ==65245== by 0x284BD2: term::terminfo::TermInfo::from_name (mod.rs:87) ==65245== by 0x2849C9: term::terminfo::TermInfo::from_env (mod.rs:73) ==65245== by 0x28F599: new (mod.rs:232) ==65245== by 0x28F599: term::stdout (lib.rs:61) ==65245== by 0x26FCBB: test::console::run_tests_console (console.rs:264) ==65245== by 0x2775E3: test::test_main (lib.rs:120) ==65245== by 0x278560: test::test_main_static (lib.rs:139) ==65245== by 0x1461F5: lib::main (in /home/cbs228/Documents/Programming/rust-netcdf/target/debug/deps/lib-28bd4a3f5ac29440) ==65245== by 0x1947FF: std::rt::lang_start::{{closure}} (rt.rs:61) ==65245== Address 0x0 is not stack'd, malloc'd or (recently) free'd ==65245== ==65245== Syscall param statx(buf) points to unaddressable byte(s) ==65245== at 0x4B0270D: syscall (syscall.S:38) ==65245== by 0x2ACFEB: statx (weak.rs:94) ==65245== by 0x2ACFEB: std::sys::unix::fs::try_statx (fs.rs:132) ==65245== by 0x2A7E0E: std::sys::unix::fs::stat (fs.rs:995) ==65245== by 0x285934: metadata<&std::path::PathBuf> (fs.rs:1553) ==65245== by 0x285934: term::terminfo::searcher::get_dbpath_for_term (searcher.rs:50) ==65245== by 0x284BD2: term::terminfo::TermInfo::from_name (mod.rs:87) ==65245== by 0x2849C9: term::terminfo::TermInfo::from_env (mod.rs:73) ==65245== by 0x28F599: new (mod.rs:232) ==65245== by 0x28F599: term::stdout (lib.rs:61) ==65245== by 0x26FCBB: test::console::run_tests_console (console.rs:264) ==65245== by 0x2775E3: test::test_main (lib.rs:120) ==65245== by 0x278560: test::test_main_static (lib.rs:139) ==65245== by 0x1461F5: lib::main (in /home/cbs228/Documents/Programming/rust-netcdf/target/debug/deps/lib-28bd4a3f5ac29440) ==65245== by 0x1947FF: std::rt::lang_start::{{closure}} (rt.rs:61) ==65245== Address 0x0 is not stack'd, malloc'd or (recently) free'd ==65245== running 44 tests test access_through_deref ... ok test add_conflicting_dimensions ... ok test add_conflicting_variables ... ok test all_var_types ... ok test append ... ok test bad_filename ... ok test count_dimensions ... ok test create ... ==65245== Invalid write of size 1 ==65245== at 0x483F0BE: strcpy (in /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so) ==65245== by 0x48A530F: nc_inq_path (in /usr/lib/x86_64-linux-gnu/libnetcdf.so.15) ==65245== by 0x2448F0: netcdf::file::File::path::{{closure}} (file.rs:121) ==65245== by 0x2302DC: netcdf::with_lock (lib.rs:128) ==65245== by 0x23C86A: netcdf::file::File::path (file.rs:120) ==65245== by 0x1300E8: lib::create (lib.rs:318) ==65245== by 0x128E09: lib::create::{{closure}} (lib.rs:313) ==65245== by 0x14C07D: core::ops::function::FnOnce::call_once (function.rs:227) ==65245== by 0x261F2E: as core::ops::function::FnOnce>::call_once (boxed.rs:942) ==65245== by 0x2AD7D9: __rust_maybe_catch_panic (lib.rs:78) ==65245== by 0x27D509: try<(),std::panic::AssertUnwindSafe>>> (panicking.rs:265) ==65245== by 0x27D509: catch_unwind>>,()> (panic.rs:396) ==65245== by 0x27D509: run_test_in_process (lib.rs:570) ==65245== by 0x27D509: test::run_test::run_test_inner::{{closure}} (lib.rs:473) ==65245== by 0x27CBE7: test::run_test::run_test_inner (lib.rs:494) ==65245== Address 0x68ef649 is 0 bytes after a block of size 25 alloc'd ==65245== at 0x483DD99: calloc (in /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so) ==65245== by 0x243BDB: alloc::alloc::alloc_zeroed (alloc.rs:165) ==65245== by 0x243A61: ::alloc_zeroed (alloc.rs:192) ==65245== by 0x2051AA: alloc::raw_vec::RawVec::allocate_in (raw_vec.rs:96) ==65245== by 0x203E38: alloc::raw_vec::RawVec::with_capacity_zeroed (raw_vec.rs:173) ==65245== by 0x23BB8C: ::from_elem (vec.rs:1715) ==65245== by 0x238205: alloc::vec::from_elem (vec.rs:1694) ==65245== by 0x23C834: netcdf::file::File::path (file.rs:118) ==65245== by 0x1300E8: lib::create (lib.rs:318) ==65245== by 0x128E09: lib::create::{{closure}} (lib.rs:313) ==65245== by 0x14C07D: core::ops::function::FnOnce::call_once (function.rs:227) ==65245== by 0x261F2E: as core::ops::function::FnOnce>::call_once (boxed.rs:942) ==65245== ok test create_group_dimensions ... ok test def_dims_vars_attrs ... ok test dimension_identifiers ... ok test dimension_identifiers_from_different_ncids ... ok test dimension_lengths ... ok test fetch_ndarray ... ok test groups_put_extra ... ok test last_dim_varies_fastest ... ok test length_of_variable ... ok test more_fill_values ... ok test nc4_groups ... ok test ndarray_read_with_indices ... ok test netcdf_error ... ok test open_to_find_unlim_dim ... ok test put_single_value ... ok test put_then_def ... ok test put_values ... ok test read_mismatched ... ok test read_slice_into_buffer ... ok test root_dims ... ok test set_compression_all_variables_in_a_group ... ok test set_fill_value ... ok test set_get_endian ... ok test single_length_variable ... ok test strided::get_to_buffer ... ok test strided::put_buffer ... ok test string_variables ... ok test test_index_fetch ... ok test unlimited_dimension_multi_putting ... ok test unlimited_dimension_single_putting ... ok test unlimited_in_parents ... ok test use_compression_chunking ... ok test use_path_to_open ... ok test use_string_to_open ... ok test var_as_different_types ... ok test variable_not_replacing ... ok test result: ok. 44 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out ```

Fix by adding a byte to the size returned by C. The NUL byte must be truncated off before the Vec is converted to String.

Good catch! Thanks a lot! Do you find valgrind normally useful when examining code written in Rust?

[cbs228]

@magnusuMET, both gdb and valgrind seem to understand the Rust debugging symbols, which is really awesome. valgrind gets quite deep into the guts of libc, and I think it can trace pretty much anything that uses dynamic symbols for malloc(), free(), memcpy(), and friends. Safe rust is designed to prevent everything that valgrind looks for, but unsafe rust—and/or calls into C libraries—is another matter. valgrind needs test-cases or other code that will cause bad writes, so tests which stress boundary conditions are always nice to have.

Most C libraries use the "buffer length" (string bytes + trailing NUL) when returning or describing size requirements for strings. netcdf-c is unusual for using only the string length.