fix(core): fix check temp path permission on mac os, fix #6256
close #6256
Will check this PR in the upcoming days but am wondering if https://github.com/tauri-apps/tauri/pull/9072/files#diff-903ee43f2eb846686e5baf219fc515ff00de27369cef1e81b37164de847d2e90 does fix the underlying issue, as it resolves symlinks now before checking. This is in dev, so not sure if it can/is/will be back ported and if other changes influence behavior.
@tweidinger I tested how path::is_symlink works, and seems it only checks the exact symbolic path, eg.
use std::{
env,
path::PathBuf,
};
fn main() {
let p_var = PathBuf::from("/var");
println!("{:?}", p_var.is_symlink()); // => true
let p_var = env::temp_dir();
println!("{:?}", p_var.is_symlink()); // => false
}
This means any subpath of a symbolic folder will return false, so https://github.com/tauri-apps/tauri/pull/9072/files#diff-903ee43f2eb846686e5baf219fc515ff00de27369cef1e81b37164de847d2e90 may not be able to fix subpath permission check.
The issue happens because the allowed path is actually a symlink so the full path does not match the path we're checking against (which is canonicalized so it resolves to the actual folder).
There is an easier way to fix this which is to canonicalize the temp dir when resolving the path variables.
basically changing https://github.com/tauri-apps/tauri/blob/44e3335da8c1a811a2ebe72f2036dfad2381da24/core/tauri/src/api/path.rs#L296 to BaseDirectory::Temp => temp_dir().canonicalize().ok()
it's probably more secure this way since we would still check canonicalized paths in the end, resolving symlinks and path traversals
cc @tweidinger
Thanks for the guidance, done!
The issue happens because the allowed path is actually a symlink so the full path does not match the path we're checking against (which is canonicalized so it resolves to the actual folder).
There is an easier way to fix this which is to canonicalize the temp dir when resolving the path variables. basically changing
https://github.com/tauri-apps/tauri/blob/44e3335da8c1a811a2ebe72f2036dfad2381da24/core/tauri/src/api/path.rs#L296
to
BaseDirectory::Temp => temp_dir().canonicalize().ok()it's probably more secure this way since we would still check canonicalized paths in the end, resolving symlinks and path traversalscc @tweidinger
When the user creates a temporary file, the js code should be like this:
import { fs, path } from '@tauri-apps/api'
import { tempdir } from '@tauri-apps/api/os'
const tempFile = await path.join(await tempdir(), 'not-exits-file')
await fs.writeFile(tempFile, 'some data')
it will use os.tempdir() to get the temporary path, when writing the file, the tempFile should not exist, so check permission will only check the path, not the canonicalized path, this will fail because os.tempdir() is not canonicalized, while the permission check is checking the canonicalized temporary path.
So, we may also change os.tempdir() API, and make it return the canonicalized temporary path.
LGTM. The only blocker is that not all commits are signed @0x-jerry could you please amend signatures to previous commits or rewrite to a single commit? The history will be squashed into a single commit anyway.
Done!