Skip to content

Commit f4c4d4f

Browse files
committed
Validate file ID for path traversal
Signed-off-by: Mateusz Szczygieł <[email protected]>
1 parent c704b34 commit f4c4d4f

File tree

2 files changed

+50
-1
lines changed

2 files changed

+50
-1
lines changed

changelog.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
### **Fresh air**
33
---
44
* Remove support for deprecated, unsecure protocols V1, V2, V4 and V5
5+
* Sanitize file ID before downloading a file, to prevent the temporary file from being allowed to traverse parent dirs
56

67
---
78
<br>

drop-transfer/src/ws/server/mod.rs

Lines changed: 49 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -880,6 +880,7 @@ impl FileXferTask {
880880
) {
881881
let task = async {
882882
validate_subpath_for_download(self.file.subpath())?;
883+
validate_file_id_for_download(self.file.id())?;
883884

884885
let emit_checksum_events = {
885886
if let Some(threshold) = state.config.checksum_events_size_threshold {
@@ -1164,9 +1165,26 @@ fn validate_subpath_for_download(subpath: &FileSubPath) -> crate::Result<()> {
11641165
Ok(())
11651166
}
11661167

1168+
/// Check file ID for illegal characters so that the temp file is not created in
1169+
/// parent directories
1170+
fn validate_file_id_for_download(file_id: &FileId) -> crate::Result<()> {
1171+
const ALLOWED_BESIDE_ALPHANUMERIC: &[char] = &['_', '-', '=']; // Add '=' just in case the padding is added
1172+
1173+
fn is_char_valid(c: char) -> bool {
1174+
c.is_ascii_alphanumeric() || ALLOWED_BESIDE_ALPHANUMERIC.contains(&c)
1175+
}
1176+
1177+
let valid = file_id.as_ref().chars().all(is_char_valid);
1178+
if !valid {
1179+
return Err(Error::BadFileId);
1180+
}
1181+
1182+
Ok(())
1183+
}
1184+
11671185
#[cfg(test)]
11681186
mod tests {
1169-
use crate::file::FileSubPath;
1187+
use crate::{file::FileSubPath, FileId};
11701188

11711189
#[test]
11721190
fn validate_subpath() {
@@ -1188,4 +1206,34 @@ mod tests {
11881206
Err(crate::Error::FilenameTooLong)
11891207
));
11901208
}
1209+
1210+
#[test]
1211+
fn validate_file_id() {
1212+
let id = FileId::from("x_ALOz5YRCXZSiS5V5Pd8VN_qqC6abyDyIBnjitzGe8");
1213+
assert!(super::validate_file_id_for_download(&id).is_ok());
1214+
1215+
let id = FileId::from("../../asdfasdf");
1216+
assert!(matches!(
1217+
super::validate_file_id_for_download(&id),
1218+
Err(crate::Error::BadFileId)
1219+
));
1220+
1221+
let id = FileId::from("..");
1222+
assert!(matches!(
1223+
super::validate_file_id_for_download(&id),
1224+
Err(crate::Error::BadFileId)
1225+
));
1226+
1227+
let id = FileId::from("/asdfasdf");
1228+
assert!(matches!(
1229+
super::validate_file_id_for_download(&id),
1230+
Err(crate::Error::BadFileId)
1231+
));
1232+
1233+
let id = FileId::from("C:\\ABC");
1234+
assert!(matches!(
1235+
super::validate_file_id_for_download(&id),
1236+
Err(crate::Error::BadFileId)
1237+
));
1238+
}
11911239
}

0 commit comments

Comments
 (0)