fix(peer): reject transfer paths outside requested game
Inbound file-transfer requests carry both a game ID and a relative path. The serve gate validated whether the requested game was currently servable, but it did not require the path itself to be rooted under that same game. A non-conforming peer could therefore register a guard for one game while asking to read files from another game root. Require normalized transfer paths to start with the requested game ID before the file can be dispatched. This keeps the outbound transfer guard, serve policy, and filesystem path aligned. Absolute, traversal, local-data, missing-sentinel, active-operation, and wrong-version paths remain rejected by the existing gates. Test Plan: - just test - just clippy - git diff --check Refs: Claude review finding #4
This commit is contained in:
@@ -218,10 +218,23 @@ async fn can_dispatch_file_transfer(
|
||||
game_id: &str,
|
||||
relative_path: &str,
|
||||
) -> bool {
|
||||
!path_points_inside_local(game_id, relative_path)
|
||||
relative_path_belongs_to_game(game_id, relative_path)
|
||||
&& !path_points_inside_local(game_id, relative_path)
|
||||
&& can_serve_game(ctx, game_dir, game_id).await
|
||||
}
|
||||
|
||||
fn relative_path_belongs_to_game(game_id: &str, relative_path: &str) -> bool {
|
||||
let normalised = relative_path.replace('\\', "/");
|
||||
if normalised.starts_with('/') {
|
||||
return false;
|
||||
}
|
||||
|
||||
normalised
|
||||
.split('/')
|
||||
.find(|part| !part.is_empty())
|
||||
.is_some_and(|first| first == game_id)
|
||||
}
|
||||
|
||||
fn path_points_inside_local(game_id: &str, relative_path: &str) -> bool {
|
||||
let normalised = relative_path.replace('\\', "/");
|
||||
let mut parts = normalised.split('/').filter(|part| !part.is_empty());
|
||||
@@ -442,6 +455,19 @@ mod tests {
|
||||
assert!(!path_points_inside_local("game", "game/archive.eti"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn transferable_paths_must_belong_to_requested_game() {
|
||||
assert!(relative_path_belongs_to_game("game", "game/version.ini"));
|
||||
assert!(relative_path_belongs_to_game("game", "game\\archive.eti"));
|
||||
assert!(!relative_path_belongs_to_game("game", "other/archive.eti"));
|
||||
assert!(!relative_path_belongs_to_game("game", "archive.eti"));
|
||||
assert!(!relative_path_belongs_to_game("game", "/game/archive.eti"));
|
||||
assert!(!relative_path_belongs_to_game(
|
||||
"game",
|
||||
"../game/archive.eti"
|
||||
));
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn get_game_response_respects_serve_gates() {
|
||||
let temp = TempDir::new("lanspread-stream");
|
||||
@@ -519,6 +545,9 @@ mod tests {
|
||||
.insert("active".to_string(), OperationKind::Downloading);
|
||||
|
||||
assert!(can_dispatch_file_transfer(&ctx, temp.path(), "ready", "ready/version.ini").await);
|
||||
assert!(
|
||||
!can_dispatch_file_transfer(&ctx, temp.path(), "ready", "active/version.ini").await
|
||||
);
|
||||
assert!(
|
||||
!can_dispatch_file_transfer(
|
||||
&ctx,
|
||||
|
||||
Reference in New Issue
Block a user