Address path traversal and other security fixes

This commit is contained in:
Cameron
2026-04-10 14:58:57 -04:00
parent e1c32b6584
commit da16fddce3
4 changed files with 19 additions and 18 deletions

View File

@@ -85,7 +85,7 @@ pub async fn login<D: UserDao>(
HttpResponse::Ok().json(Token { token: &token }) HttpResponse::Ok().json(Token { token: &token })
} else { } else {
error!("Failed login attempt for user: '{}'", creds.username); error!("Failed login attempt for user: '{}'", creds.username);
HttpResponse::NotFound().finish() HttpResponse::Unauthorized().finish()
} }
} }
@@ -128,7 +128,7 @@ mod tests {
} }
#[actix_rt::test] #[actix_rt::test]
async fn test_login_reports_404_when_user_does_not_exist() { async fn test_login_reports_401_when_user_does_not_exist() {
let mut dao = TestUserDao::new(); let mut dao = TestUserDao::new();
dao.create_user("user", "password"); dao.create_user("user", "password");
@@ -139,6 +139,6 @@ mod tests {
let response = login::<TestUserDao>(j, web::Data::new(Mutex::new(dao))).await; let response = login::<TestUserDao>(j, web::Data::new(Mutex::new(dao))).await;
assert_eq!(response.status(), 404); assert_eq!(response.status(), 401);
} }
} }

View File

@@ -85,7 +85,7 @@ impl FromRequest for Claims {
) )
.and_then(|header| { .and_then(|header| {
Claims::from_str(header) Claims::from_str(header)
.with_context(|| format!("Unable to decode token from: {}", header)) .with_context(|| "Unable to decode token from Authorization header")
}) })
.map_or_else( .map_or_else(
|e| { |e| {

View File

@@ -932,6 +932,7 @@ pub async fn get_gps_summary(
request: HttpRequest, request: HttpRequest,
req: Query<FilesRequest>, req: Query<FilesRequest>,
exif_dao: Data<Mutex<Box<dyn ExifDao>>>, exif_dao: Data<Mutex<Box<dyn ExifDao>>>,
app_state: Data<AppState>,
) -> Result<HttpResponse, actix_web::Error> { ) -> Result<HttpResponse, actix_web::Error> {
use crate::data::{GpsPhotoSummary, GpsPhotosResponse}; use crate::data::{GpsPhotoSummary, GpsPhotosResponse};
@@ -952,17 +953,17 @@ pub async fn get_gps_summary(
// The database stores relative paths, so we use the path as-is // The database stores relative paths, so we use the path as-is
// Normalize empty path or "/" to return all GPS photos // Normalize empty path or "/" to return all GPS photos
let requested_path = if req.path.is_empty() || req.path == "/" { let requested_path = if req.path.is_empty() || req.path == "/" {
"" String::new()
} else { } else {
// Just do basic validation to prevent path traversal // Validate path using the same check as all other endpoints
if req.path.contains("..") { if is_valid_full_path(&app_state.base_path, &req.path, false).is_none() {
warn!("Path traversal attempt: {}", req.path); warn!("Invalid path for GPS summary: {}", req.path);
cx.span().set_status(Status::error("Invalid path")); cx.span().set_status(Status::error("Invalid path"));
return Ok(HttpResponse::Forbidden().json(serde_json::json!({ return Ok(HttpResponse::BadRequest().json(serde_json::json!({
"error": "Invalid path" "error": "Invalid path"
}))); })));
} }
req.path.as_str() req.path.clone()
}; };
let recursive = req.recursive.unwrap_or(false); let recursive = req.recursive.unwrap_or(false);
@@ -973,7 +974,7 @@ pub async fn get_gps_summary(
// Query database for all photos with GPS // Query database for all photos with GPS
let mut exif_dao_guard = exif_dao.lock().expect("Unable to get ExifDao"); let mut exif_dao_guard = exif_dao.lock().expect("Unable to get ExifDao");
match exif_dao_guard.get_all_with_gps(&cx, requested_path, recursive) { match exif_dao_guard.get_all_with_gps(&cx, &requested_path, recursive) {
Ok(gps_data) => { Ok(gps_data) => {
let mut photos: Vec<GpsPhotoSummary> = gps_data let mut photos: Vec<GpsPhotoSummary> = gps_data
.into_iter() .into_iter()

View File

@@ -503,14 +503,10 @@ async fn stream_video(
let playlist = &path.path; let playlist = &path.path;
debug!("Playlist: {}", playlist); debug!("Playlist: {}", playlist);
// Extract video playlist dir to dotenv // Only serve files under video_path (HLS playlists) or base_path (source videos)
if !playlist.starts_with(&app_state.video_path) if playlist.starts_with(&app_state.video_path)
&& is_valid_full_path(&app_state.base_path, playlist, false).is_some() || is_valid_full_path(&app_state.base_path, playlist, false).is_some()
{ {
span.set_status(Status::error(format!("playlist not valid {}", playlist)));
HttpResponse::BadRequest().finish()
} else {
match NamedFile::open(playlist) { match NamedFile::open(playlist) {
Ok(file) => { Ok(file) => {
span.set_status(Status::Ok); span.set_status(Status::Ok);
@@ -521,6 +517,9 @@ async fn stream_video(
HttpResponse::NotFound().finish() HttpResponse::NotFound().finish()
} }
} }
} else {
span.set_status(Status::error(format!("playlist not valid {}", playlist)));
HttpResponse::BadRequest().finish()
} }
} }
@@ -1209,6 +1208,7 @@ fn main() -> std::io::Result<()> {
.app_data::<Data<Mutex<SqliteKnowledgeDao>>>(Data::new(Mutex::new( .app_data::<Data<Mutex<SqliteKnowledgeDao>>>(Data::new(Mutex::new(
SqliteKnowledgeDao::new(), SqliteKnowledgeDao::new(),
))) )))
.app_data(mp::form::MultipartFormConfig::default().total_limit(1024 * 1024 * 1024)) // 1GB upload limit
.app_data(web::JsonConfig::default().error_handler(|err, req| { .app_data(web::JsonConfig::default().error_handler(|err, req| {
let detail = err.to_string(); let detail = err.to_string();
log::warn!( log::warn!(