cjw/Oxicloud
1
0
Fork 0
Code Issues Pull Requests Actions 1 Packages Projects Releases Wiki Activity

fix(security): V-01,V-02,V-04,V-05,V-06 - multiple vulnerability fixes

Browse Source 
- V-01: CalDAV unbounded body limit → MAX_CALDAV_BODY = 1MB
- V-02: CardDAV unbounded body limit → MAX_CARDDAV_BODY = 1MB
- V-04: Batch operations without max size → MAX_BATCH_SIZE = 1000
- V-05: WOPI get_editor_url IDOR → authorize_wopi_access with ownership check
- V-06: JWT secret without entropy validation → panic <16, warn 16-31, accept >=32
This commit is contained in:
Dionisio
2026-03-05 16:57:44 +01:00
parent 4197cc3b7b
commit 4d81bcbd7b
5 changed files with 176 additions and 37 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/common/config.rs
+21 -1
View File
@@ -569,10 +569,30 @@ impl AppConfig {
// Auth configuration
if let Ok(jwt_secret) = env::var("OXICLOUD_JWT_SECRET") {
// SECURITY: Validate JWT secret minimum entropy (RFC 7518 §3.2
// recommends ≥256 bits for HS256). Panic on dangerously short
// secrets, warn on sub-optimal ones.
let len = jwt_secret.len();
if config.features.enable_auth && len < 16 {
panic!(
"FATAL: OXICLOUD_JWT_SECRET is dangerously short ({} bytes). \
Minimum: 32 bytes (256 bits) for HS256. \
Generate a secure secret with: openssl rand -hex 32",
len
);
} else if config.features.enable_auth && len < 32 {
tracing::warn!("==========================================================");
tracing::warn!(
"OXICLOUD_JWT_SECRET is only {} bytes — recommended minimum is 32 (256 bits).",
len
);
tracing::warn!("Generate a stronger secret with: openssl rand -hex 32");
tracing::warn!("==========================================================");
}
config.auth.jwt_secret = jwt_secret;
}
// SECURITY: Validate JWT secret when auth is enabled
// SECURITY: Generate ephemeral secret when none is provided
if config.features.enable_auth && config.auth.jwt_secret.is_empty() {
// Generate a random secret for this session and warn loudly
use rand_core::{OsRng, RngCore};
src/interfaces/api/handlers/batch_handler.rs
+93
View File
@@ -14,6 +14,10 @@ use crate::application::services::batch_operations::{
use crate::interfaces::api::handlers::ApiResult;
use crate::interfaces::middleware::auth::AuthUser;
/// Maximum number of items allowed in a single batch request.
/// Prevents fan-out amplification attacks and database connection exhaustion.
const MAX_BATCH_SIZE: usize = 1_000;
/// Shared state for the batch handler
#[derive(Clone)]
pub struct BatchHandlerState {
@@ -143,6 +147,15 @@ pub async fn move_files_batch(
)
.into_response());
}
if request.file_ids.len() > MAX_BATCH_SIZE {
return Ok((
StatusCode::BAD_REQUEST,
Json(serde_json::json!({
"error": format!("Batch size {} exceeds maximum of {}", request.file_ids.len(), MAX_BATCH_SIZE)
})),
)
.into_response());
}
// Execute batch operation
let result = state
@@ -187,6 +200,15 @@ pub async fn copy_files_batch(
)
.into_response());
}
if request.file_ids.len() > MAX_BATCH_SIZE {
return Ok((
StatusCode::BAD_REQUEST,
Json(serde_json::json!({
"error": format!("Batch size {} exceeds maximum of {}", request.file_ids.len(), MAX_BATCH_SIZE)
})),
)
.into_response());
}
// Execute batch operation
let result = state
@@ -231,6 +253,15 @@ pub async fn delete_files_batch(
)
.into_response());
}
if request.file_ids.len() > MAX_BATCH_SIZE {
return Ok((
StatusCode::BAD_REQUEST,
Json(serde_json::json!({
"error": format!("Batch size {} exceeds maximum of {}", request.file_ids.len(), MAX_BATCH_SIZE)
})),
)
.into_response());
}
// Execute batch operation
let result = state
@@ -283,6 +314,15 @@ pub async fn delete_folders_batch(
)
.into_response());
}
if request.folder_ids.len() > MAX_BATCH_SIZE {
return Ok((
StatusCode::BAD_REQUEST,
Json(serde_json::json!({
"error": format!("Batch size {} exceeds maximum of {}", request.folder_ids.len(), MAX_BATCH_SIZE)
})),
)
.into_response());
}
// Execute batch operation
let result = state
@@ -335,6 +375,15 @@ pub async fn create_folders_batch(
)
.into_response());
}
if request.folders.len() > MAX_BATCH_SIZE {
return Ok((
StatusCode::BAD_REQUEST,
Json(serde_json::json!({
"error": format!("Batch size {} exceeds maximum of {}", request.folders.len(), MAX_BATCH_SIZE)
})),
)
.into_response());
}
// Transform the format for the service
let folders = request
@@ -386,6 +435,15 @@ pub async fn get_files_batch(
)
.into_response());
}
if request.file_ids.len() > MAX_BATCH_SIZE {
return Ok((
StatusCode::BAD_REQUEST,
Json(serde_json::json!({
"error": format!("Batch size {} exceeds maximum of {}", request.file_ids.len(), MAX_BATCH_SIZE)
})),
)
.into_response());
}
// Execute batch operation
let result = state
@@ -430,6 +488,15 @@ pub async fn get_folders_batch(
)
.into_response());
}
if request.folder_ids.len() > MAX_BATCH_SIZE {
return Ok((
StatusCode::BAD_REQUEST,
Json(serde_json::json!({
"error": format!("Batch size {} exceeds maximum of {}", request.folder_ids.len(), MAX_BATCH_SIZE)
})),
)
.into_response());
}
// Execute batch operation
let result = state
@@ -495,6 +562,16 @@ pub async fn trash_batch(
)
.into_response());
}
let combined_size = request.file_ids.len() + request.folder_ids.len();
if combined_size > MAX_BATCH_SIZE {
return Ok((
StatusCode::BAD_REQUEST,
Json(serde_json::json!({
"error": format!("Batch size {} exceeds maximum of {}", combined_size, MAX_BATCH_SIZE)
})),
)
.into_response());
}
let mut all_successful: Vec<String> = Vec::new();
let mut all_failed: Vec<FailedOperation> = Vec::new();
@@ -597,6 +674,15 @@ pub async fn move_folders_batch(
)
.into_response());
}
if request.folder_ids.len() > MAX_BATCH_SIZE {
return Ok((
StatusCode::BAD_REQUEST,
Json(serde_json::json!({
"error": format!("Batch size {} exceeds maximum of {}", request.folder_ids.len(), MAX_BATCH_SIZE)
})),
)
.into_response());
}
let result = state
.batch_service
@@ -637,6 +723,13 @@ pub async fn download_batch(
"No file or folder IDs provided".to_string(),
));
}
let combined_size = request.file_ids.len() + request.folder_ids.len();
if combined_size > MAX_BATCH_SIZE {
return Err((
StatusCode::BAD_REQUEST,
format!("Batch size {} exceeds maximum of {}", combined_size, MAX_BATCH_SIZE),
));
}
let temp_file = state
.batch_service
src/interfaces/api/handlers/caldav_handler.rs
+9 -5
View File
@@ -39,6 +39,10 @@ use crate::interfaces::middleware::auth::CurrentUser;
const HEADER_DAV: HeaderName = HeaderName::from_static("dav");
/// Maximum allowed request body size for CalDAV XML/iCal endpoints (1 MB).
/// Prevents OOM/DoS via unbounded body buffering.
const MAX_CALDAV_BODY: usize = 1_048_576;
/// Creates CalDAV routes with full path prefixes.
///
/// Uses `merge()` instead of `nest()` to avoid Axum's trailing-slash routing gap.
@@ -182,7 +186,7 @@ async fn handle_propfind(
let user = extract_user(&req)?;
let calendar_service = get_calendar_service(&state)?;
let body_bytes = body::to_bytes(req.into_body(), usize::MAX)
let body_bytes = body::to_bytes(req.into_body(), MAX_CALDAV_BODY)
.await
.map_err(|e| AppError::bad_request(format!("Failed to read request body: {}", e)))?;
@@ -423,7 +427,7 @@ async fn handle_report(
let user = extract_user(&req)?;
let calendar_service = get_calendar_service(&state)?;
let body_bytes = body::to_bytes(req.into_body(), usize::MAX)
let body_bytes = body::to_bytes(req.into_body(), MAX_CALDAV_BODY)
.await
.map_err(|e| AppError::bad_request(format!("Failed to read request body: {}", e)))?;
@@ -498,7 +502,7 @@ async fn handle_mkcalendar(
let user = extract_user(&req)?;
let calendar_service = get_calendar_service(&state)?;
let body_bytes = body::to_bytes(req.into_body(), usize::MAX)
let body_bytes = body::to_bytes(req.into_body(), MAX_CALDAV_BODY)
.await
.map_err(|e| AppError::bad_request(format!("Failed to read request body: {}", e)))?;
@@ -551,7 +555,7 @@ async fn handle_put(
let calendar_id = parts[0];
let body_bytes = body::to_bytes(req.into_body(), usize::MAX)
let body_bytes = body::to_bytes(req.into_body(), MAX_CALDAV_BODY)
.await
.map_err(|e| AppError::bad_request(format!("Failed to read request body: {}", e)))?;
@@ -795,7 +799,7 @@ async fn handle_proppatch(
let user = extract_user(&req)?;
let calendar_service = get_calendar_service(&state)?;
let body_bytes = body::to_bytes(req.into_body(), usize::MAX)
let body_bytes = body::to_bytes(req.into_body(), MAX_CALDAV_BODY)
.await
.map_err(|e| AppError::bad_request(format!("Failed to read request body: {}", e)))?;
src/interfaces/api/handlers/carddav_handler.rs
+9 -5
View File
@@ -39,6 +39,10 @@ use crate::interfaces::middleware::auth::CurrentUser;
const HEADER_DAV: HeaderName = HeaderName::from_static("dav");
/// Maximum allowed request body size for CardDAV XML/vCard endpoints (1 MB).
/// Prevents OOM/DoS via unbounded body buffering.
const MAX_CARDDAV_BODY: usize = 1_048_576;
/// Creates CardDAV routes with full path prefixes.
///
/// Uses `merge()` instead of `nest()` to avoid Axum's trailing-slash routing gap.
@@ -181,7 +185,7 @@ async fn handle_propfind(
let addressbook_service = get_addressbook_service(&state)?;
let contact_svc = get_contact_service(&state)?;
let body_bytes = body::to_bytes(req.into_body(), usize::MAX)
let body_bytes = body::to_bytes(req.into_body(), MAX_CARDDAV_BODY)
.await
.map_err(|e| AppError::bad_request(format!("Failed to read request body: {}", e)))?;
@@ -312,7 +316,7 @@ async fn handle_report(
let user = extract_user(&req)?;
let contact_svc = get_contact_service(&state)?;
let body_bytes = body::to_bytes(req.into_body(), usize::MAX)
let body_bytes = body::to_bytes(req.into_body(), MAX_CARDDAV_BODY)
.await
.map_err(|e| AppError::bad_request(format!("Failed to read request body: {}", e)))?;
@@ -381,7 +385,7 @@ async fn handle_mkcol(
let user = extract_user(&req)?;
let addressbook_service = get_addressbook_service(&state)?;
let body_bytes = body::to_bytes(req.into_body(), usize::MAX)
let body_bytes = body::to_bytes(req.into_body(), MAX_CARDDAV_BODY)
.await
.map_err(|e| AppError::bad_request(format!("Failed to read request body: {}", e)))?;
@@ -435,7 +439,7 @@ async fn handle_put(
let address_book_id = parts[0];
let body_bytes = body::to_bytes(req.into_body(), usize::MAX)
let body_bytes = body::to_bytes(req.into_body(), MAX_CARDDAV_BODY)
.await
.map_err(|e| AppError::bad_request(format!("Failed to read request body: {}", e)))?;
@@ -628,7 +632,7 @@ async fn handle_proppatch(
let user = extract_user(&req)?;
let addressbook_service = get_addressbook_service(&state)?;
let body_bytes = body::to_bytes(req.into_body(), usize::MAX)
let body_bytes = body::to_bytes(req.into_body(), MAX_CARDDAV_BODY)
.await
.map_err(|e| AppError::bad_request(format!("Failed to read request body: {}", e)))?;
src/interfaces/api/handlers/wopi_handler.rs
+44 -26
View File
@@ -386,6 +386,28 @@ pub struct EditorUrlResponse {
pub access_token_ttl: i64,
}
/// Determines if `caller_id` can access `file_id` and with what permissions.
///
/// Uses the SQL-level ownership check (`get_file_owned`) so that files
/// belonging to other users — or non-existent files — both return `NOT_FOUND`,
/// avoiding existence-leak oracles.
///
/// Returns `(FileDto, can_write)` on success.
async fn authorize_wopi_access<S: FileRetrievalUseCase>(
file_retrieval: &S,
file_id: &str,
caller_id: &str,
requested_action: &str,
) -> Result<(crate::application::dtos::file_dto::FileDto, bool), StatusCode> {
let file = file_retrieval
.get_file_owned(file_id, caller_id)
.await
.map_err(|_| StatusCode::NOT_FOUND)?;
// Owner verified — grant write unless explicitly requesting view-only.
let can_write = requested_action != "view";
Ok((file, can_write))
}
/// GET /api/wopi/editor-url — Returns the editor iframe URL + WOPI token.
///
/// This endpoint is behind normal auth middleware. The authenticated user
@@ -399,16 +421,17 @@ pub async fn get_editor_url(
Query(params): Query<EditorUrlParams>,
State(state): State<WopiState>,
) -> Response {
// Get file info to determine extension
let file = match state
.app_state
.applications
.file_retrieval_service
.get_file(&params.file_id)
.await
// Verify the caller owns the file (SQL-level check, no existence leak).
let (file, can_write) = match authorize_wopi_access(
state.app_state.applications.file_retrieval_service.as_ref(),
&params.file_id,
&user_id,
&params.action,
)
.await
{
Ok(f) => f,
Err(_) => return StatusCode::NOT_FOUND.into_response(),
Ok(result) => result,
Err(status) => return status.into_response(),
};
// Extract extension from filename
@@ -437,13 +460,6 @@ pub async fn get_editor_url(
}
};
// Determine write permission: owner can write, others read-only.
// If no owner_id on the file, default to allowing write.
let can_write = match &file.owner_id {
Some(owner) => owner == &user_id,
None => true,
};
// Generate WOPI access token
let (access_token, access_token_ttl) =
match state
@@ -485,20 +501,22 @@ async fn host_page(
return StatusCode::UNAUTHORIZED.into_response();
}
// Get file info for extension
let file = match state
.app_state
.applications
.file_retrieval_service
.get_file(&file_id)
.await
// Re-verify ownership even though the token was valid — defence in depth.
let requested_action = if claims.can_write { "edit" } else { "view" };
let file = match authorize_wopi_access(
state.app_state.applications.file_retrieval_service.as_ref(),
&file_id,
&claims.sub,
requested_action,
)
.await
{
Ok(f) => f,
Err(_) => return StatusCode::NOT_FOUND.into_response(),
Ok((f, _)) => f,
Err(status) => return status.into_response(),
};
let extension = file.name.rsplit('.').next().unwrap_or("").to_lowercase();
let action = if claims.can_write { "edit" } else { "view" };
let action = requested_action;
let wopi_src = format!("{}/wopi/files/{}", state.wopi_base_url, file_id);
let editor_url = match state