Skip to content

Commit d39807e

Browse files
Ignacio Van Droogenbroeckclaude
andcommitted
fix(security): restrict temp file permissions and validate memory_limit
- chmod 0600 on CreateTemp files in restore, delete, and duckdb profiling to prevent world-readable parquet data and profile output (closes #362) - validate memory_limit with regex before SQL interpolation in compaction subprocess to prevent injection via config (closes #363) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent 3dfac9d commit d39807e

4 files changed

Lines changed: 27 additions & 0 deletions

File tree

internal/api/delete.go

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -623,6 +623,11 @@ func (h *DeleteHandler) rewriteS3File(ctx context.Context, s3Path, relativePath,
623623
if err != nil {
624624
return 0, fmt.Errorf("failed to create temp file: %w", err)
625625
}
626+
if err := os.Chmod(tempFile.Name(), 0600); err != nil {
627+
tempFile.Close()
628+
os.Remove(tempFile.Name())
629+
return 0, fmt.Errorf("failed to secure temp file: %w", err)
630+
}
626631
tempPath := tempFile.Name()
627632
tempFile.Close()
628633
defer os.Remove(tempPath)

internal/backup/restore.go

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -146,6 +146,11 @@ func (m *Manager) streamRestoreFile(ctx context.Context, srcPath, destPath strin
146146
if err != nil {
147147
return 0, fmt.Errorf("failed to create temp file: %w", err)
148148
}
149+
if err := os.Chmod(tmpFile.Name(), 0600); err != nil {
150+
tmpFile.Close()
151+
os.Remove(tmpFile.Name())
152+
return 0, fmt.Errorf("failed to secure temp file: %w", err)
153+
}
149154
tmpPath := tmpFile.Name()
150155
defer os.Remove(tmpPath)
151156
defer tmpFile.Close()

internal/compaction/subprocess.go

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import (
99
"os"
1010
"os/exec"
1111
"os/signal"
12+
"regexp"
1213
"runtime"
1314
"runtime/pprof"
1415
"strings"
@@ -21,6 +22,8 @@ import (
2122
_ "github.com/duckdb/duckdb-go/v2"
2223
)
2324

25+
var memLimitRe = regexp.MustCompile(`^\d+(\.\d+)?\s*(B|KB|MB|GB|TB|%)?$`)
26+
2427
// SubprocessJobConfig is the serializable configuration passed to the subprocess.
2528
// It contains all information needed to run a compaction job in isolation.
2629
type SubprocessJobConfig struct {
@@ -90,6 +93,9 @@ func RunSubprocessJob(config *SubprocessJobConfig) (*SubprocessJobResult, error)
9093
// Set DuckDB memory limit from config.
9194
// This prevents OOM on servers without swap by forcing DuckDB to spill to disk.
9295
if config.MemoryLimit != "" {
96+
if !memLimitRe.MatchString(config.MemoryLimit) {
97+
return nil, fmt.Errorf("invalid memory_limit value: %q", config.MemoryLimit)
98+
}
9399
if _, err := db.Exec(fmt.Sprintf("SET memory_limit='%s'", config.MemoryLimit)); err != nil {
94100
logger.Warn().Err(err).Str("limit", config.MemoryLimit).Msg("Failed to set DuckDB memory limit")
95101
} else {

internal/database/duckdb.go

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -560,6 +560,17 @@ func (d *DuckDB) QueryWithProfileContext(ctx context.Context, query string) (*sq
560560
}
561561
return rows, conn, nil, nil
562562
}
563+
if err := os.Chmod(tmpFile.Name(), 0600); err != nil {
564+
tmpFile.Close()
565+
os.Remove(tmpFile.Name())
566+
// Fall back to regular query
567+
rows, err := conn.QueryContext(ctx, query)
568+
if err != nil {
569+
conn.Close()
570+
return nil, nil, nil, err
571+
}
572+
return rows, conn, nil, nil
573+
}
563574
profilePath := tmpFile.Name()
564575
tmpFile.Close()
565576
defer os.Remove(profilePath)

0 commit comments

Comments
 (0)