diff --git a/internal/commands/attachment.go b/internal/commands/attachment.go index 6aa6430..50e3907 100644 --- a/internal/commands/attachment.go +++ b/internal/commands/attachment.go @@ -1,6 +1,7 @@ package commands import ( + "path/filepath" "regexp" "strconv" @@ -112,7 +113,8 @@ Use 'fizzy card attachments show CARD_NUMBER' to see available attachments and t // Download the files var results []map[string]interface{} for _, attachment := range toDownload { - outputPath := attachment.Filename + // Sanitize filename to prevent path traversal attacks + outputPath := filepath.Base(attachment.Filename) // If downloading single file with custom output name if len(toDownload) == 1 && attachmentDownloadOutput != "" { outputPath = attachmentDownloadOutput diff --git a/internal/commands/attachment_test.go b/internal/commands/attachment_test.go index 1bb6cd9..d2023fa 100644 --- a/internal/commands/attachment_test.go +++ b/internal/commands/attachment_test.go @@ -415,3 +415,84 @@ func stringContains(s, substr string) bool { } return false } + +func TestAttachmentDownloadSanitizesFilename(t *testing.T) { + tests := []struct { + name string + maliciousFilename string + expectedOutputPath string + }{ + { + name: "path traversal with ../", + maliciousFilename: "../../../etc/passwd", + expectedOutputPath: "passwd", + }, + { + name: "path traversal with subdirectory", + maliciousFilename: "../../.bashrc", + expectedOutputPath: ".bashrc", + }, + { + name: "absolute path attempt", + maliciousFilename: "/etc/shadow", + expectedOutputPath: "shadow", + }, + { + name: "normal filename unchanged", + maliciousFilename: "safe-file.png", + expectedOutputPath: "safe-file.png", + }, + { + name: "filename with spaces", + maliciousFilename: "my document.pdf", + expectedOutputPath: "my document.pdf", + }, + { + name: "deeply nested traversal", + maliciousFilename: "../../../../../../../../tmp/malware.sh", + expectedOutputPath: "malware.sh", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + cardData := map[string]interface{}{ + "id": "card-id", + "number": 241, + "description_html": ` + Download + `, + } + + mock := NewMockClient().WithGetData(cardData) + result := SetTestMode(mock) + SetTestConfig("test-token", "test-account", "https://api.test.com") + defer ResetTestMode() + + rootCmd.SetArgs([]string{"card", "attachments", "download", "241", "1"}) + + RunTestCommand(func() { + _ = rootCmd.Execute() + }) + + if result.Response == nil { + t.Fatal("expected response, got nil") + } + + if !result.Response.Success { + t.Errorf("expected success, got error: %v", result.Response) + return + } + + // Verify the output path was sanitized + if len(mock.DownloadFileCalls) != 1 { + t.Fatalf("expected 1 download call, got %d", len(mock.DownloadFileCalls)) + } + + actualOutputPath := mock.DownloadFileCalls[0].DestPath + if actualOutputPath != tt.expectedOutputPath { + t.Errorf("expected sanitized output path %q, got %q", tt.expectedOutputPath, actualOutputPath) + } + }) + } +}