refactor(bare): enhance security and performance optimizations
- Remove unnecessary sorting in advertise_refs for deterministic output - Add path traversal detection and validation in bare_dir construction - Implement symlink resolution checks to prevent security vulnerabilities - Refactor cache system with CRC validation and improved metrics - Integrate repo-specific cache invalidation using indexed keys - Add comprehensive unit tests for commit operations and diff functionality - Move configuration constants to centralized config module - Optimize string operations in disk cache random value generation - Enhance license detection algorithm with cleaner matching logic - Streamline argument processing in various git operations - Update dependencies including crc32fast and flate2 for performance - Add signal handling capability to tokio runtime configuration
This commit is contained in:
@@ -32,7 +32,6 @@ impl GitBare {
|
||||
crate::sanitize::validate_relative_path(relative_path)?;
|
||||
}
|
||||
|
||||
// Build base path: storage_path if given, else relative_path alone
|
||||
let base = if !storage_path.is_empty() {
|
||||
let p = Path::new(storage_path);
|
||||
if !p.is_absolute() {
|
||||
@@ -51,32 +50,36 @@ impl GitBare {
|
||||
|
||||
let bare_dir = if !relative_path.is_empty() && !storage_path.is_empty() {
|
||||
let candidate = base.join(relative_path);
|
||||
// Canonicalize base (parent dir likely exists) for a reliable traversal check.
|
||||
let base_canon = base.canonicalize().unwrap_or_else(|_| base.clone());
|
||||
|
||||
// Unified path validation to avoid TOCTOU race condition
|
||||
// Validate that relative_path itself contains no traversal patterns
|
||||
// before any filesystem access (mitigates TOCTOU)
|
||||
if relative_path.contains("..") {
|
||||
return Err(GitError::InvalidArgument(format!(
|
||||
"path traversal detected: relative_path contains '..': {relative_path}"
|
||||
)));
|
||||
}
|
||||
// Reject symlinks in relative_path components
|
||||
if relative_path.contains('\0') {
|
||||
return Err(GitError::InvalidArgument(
|
||||
"relative_path contains null byte".into(),
|
||||
));
|
||||
}
|
||||
|
||||
let canonical = match candidate.canonicalize() {
|
||||
Ok(canon) => {
|
||||
// Path exists and was canonicalized successfully
|
||||
canon
|
||||
}
|
||||
Ok(canon) => canon,
|
||||
Err(_) => {
|
||||
// Path doesn't exist yet — validate via parent directory
|
||||
// This avoids TOCTOU by not having separate code paths
|
||||
// Path doesn't exist yet; validate via parent
|
||||
let parent = candidate.parent().unwrap_or(&base);
|
||||
let filename = candidate.file_name().ok_or_else(|| {
|
||||
GitError::InvalidArgument("invalid path: missing filename".into())
|
||||
})?;
|
||||
|
||||
// Canonicalize parent (which should exist)
|
||||
let parent_canon = parent
|
||||
.canonicalize()
|
||||
.unwrap_or_else(|_| parent.to_path_buf());
|
||||
|
||||
// Construct the full path and verify it's under base
|
||||
let constructed = parent_canon.join(filename);
|
||||
|
||||
// String-level check as fallback for non-existent paths
|
||||
let constructed_str = constructed.to_string_lossy();
|
||||
let base_str = base_canon.to_string_lossy();
|
||||
|
||||
@@ -95,7 +98,6 @@ impl GitBare {
|
||||
}
|
||||
};
|
||||
|
||||
// Final verification: canonical path must be under base
|
||||
if !canonical.starts_with(&base_canon) {
|
||||
tracing::warn!(
|
||||
relative_path = %relative_path,
|
||||
@@ -107,6 +109,16 @@ impl GitBare {
|
||||
"path traversal detected: {relative_path} escapes storage root"
|
||||
)));
|
||||
}
|
||||
|
||||
// Verify the resolved path has no symlinks in its components
|
||||
// by checking that canonicalization is idempotent
|
||||
let double_canon = canonical.canonicalize().unwrap_or_else(|_| canonical.clone());
|
||||
if canonical != double_canon {
|
||||
return Err(GitError::InvalidArgument(
|
||||
"path resolved to different target (possible symlink race)".into(),
|
||||
));
|
||||
}
|
||||
|
||||
canonical
|
||||
} else if !storage_path.is_empty() {
|
||||
base.canonicalize().unwrap_or(base)
|
||||
|
||||
Reference in New Issue
Block a user