Skip to content

BUG: Path traversal vulnerability in attachment download #725

@andrinoff

Description

@andrinoff

Describe the bug
In main.go line 2611, attachment filename from email headers (untrusted input) is used directly in filepath.Join() without sanitization. Malicious emails with filenames like "../../../.ssh/authorized_keys" could write files outside Downloads directory.

To reproduce

  1. Examine main.go lines 2603-2611:
origName := msg.Filename  // From email attachment header - untrusted!
ext := filepath.Ext(origName)
base := strings.TrimSuffix(origName, ext)
candidate := origName
// ...
for {
    filePath = filepath.Join(downloadsPath, candidate)
    // ... write to filePath
}
  1. Send email with attachment named: Content-Disposition: attachment; filename="../../../.ssh/authorized_keys"

  2. While filepath.Join() cleans some paths, it doesn't prevent traversal if filename starts with ../

  3. File could be written outside Downloads directory

Expected behavior
Sanitize filename to prevent directory traversal:

  • Strip all path separators (/ and \)
  • Use filepath.Base() to extract only filename component
  • Validate filename doesn't contain dangerous characters
  • Reject filenames starting with . (hidden files)

Additional context

  • Good first issue: straightforward input sanitization
  • Security issue: path traversal could overwrite arbitrary user files
  • Attack requires victim to download malicious attachment
  • filepath.Join("/home/user/Downloads", "../../../etc/passwd") = "/etc/passwd" on some systems
  • Similar issue could affect other file operations

Suggested fix:

// Sanitize attachment filename to prevent path traversal
func sanitizeFilename(name string) string {
    // Use only the base filename, stripping any path components
    name = filepath.Base(name)
    
    // Remove any remaining path separators (in case Base() doesn't catch all)
    name = strings.ReplaceAll(name, "/", "_")
    name = strings.ReplaceAll(name, "\\", "_")
    name = strings.ReplaceAll(name, "..", "_")
    
    // If empty or starts with dot, use default
    if name == "" || name == "." || strings.HasPrefix(name, ".") {
        name = "attachment"
    }
    
    return name
}

// In downloadAttachmentCmd:
origName := sanitizeFilename(msg.Filename)

OS
All platforms

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't workinggood first issueGood for newcomers

    Type

    No type

    Projects

    Status

    Done

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions