improve security

Author: eljojo

internal/cmd/init.go

@@ -35,6 +35,15 @@ var (
 	initFriends   []string
 )
 
+const (
+	// MaxNameLength is the maximum allowed length for friend names
+	MaxNameLength = 200
+	// MaxEmailLength is the maximum allowed length for email addresses
+	MaxEmailLength = 320 // RFC 5321 maximum
+	// MaxPhoneLength is the maximum allowed length for phone numbers
+	MaxPhoneLength = 50
+)
+
 func init() {
 	rootCmd.AddCommand(initCmd)
 	initCmd.Flags().StringVar(&initFrom, "from", "", "Base new project on existing project (copies friends)")
@@ -155,6 +164,9 @@ func runInit(cmd *cobra.Command, args []string) error {
 			if nameStr == "" {
 				return fmt.Errorf("name is required")
 			}
+			if len(nameStr) > MaxNameLength {
+				return fmt.Errorf("name too long (max %d characters)", MaxNameLength)
+			}
 			friends[i].Name = nameStr
 
 			fmt.Print("  Email: ")
@@ -163,11 +175,18 @@ func runInit(cmd *cobra.Command, args []string) error {
 			if emailStr == "" {
 				return fmt.Errorf("email is required")
 			}
+			if len(emailStr) > MaxEmailLength {
+				return fmt.Errorf("email too long (max %d characters)", MaxEmailLength)
+			}
 			friends[i].Email = emailStr
 
 			fmt.Print("  Phone (optional): ")
 			phoneStr, _ := reader.ReadString('\n')
-			friends[i].Phone = strings.TrimSpace(phoneStr)
+			phoneStr = strings.TrimSpace(phoneStr)
+			if len(phoneStr) > MaxPhoneLength {
+				return fmt.Errorf("phone too long (max %d characters)", MaxPhoneLength)
+			}
+			friends[i].Phone = phoneStr
 
 			fmt.Println()
 		}
@@ -226,9 +245,18 @@ func parseFriendFlags(flags []string) ([]project.Friend, error) {
 		if friends[i].Name == "" {
 			return nil, fmt.Errorf("friend name cannot be empty")
 		}
+		if len(friends[i].Name) > MaxNameLength {
+			return nil, fmt.Errorf("friend name too long (max %d characters)", MaxNameLength)
+		}
 		if friends[i].Email == "" {
 			return nil, fmt.Errorf("friend email cannot be empty")
 		}
+		if len(friends[i].Email) > MaxEmailLength {
+			return nil, fmt.Errorf("friend email too long (max %d characters)", MaxEmailLength)
+		}
+		if len(friends[i].Phone) > MaxPhoneLength {
+			return nil, fmt.Errorf("friend phone too long (max %d characters)", MaxPhoneLength)
+		}
 	}
 	return friends, nil
 }

internal/crypto/age.go

@@ -1,15 +1,22 @@
 package crypto
 
 import (
+	"errors"
 	"fmt"
 	"io"
 
 	"filippo.io/age"
 )
 
+// ErrEmptyPassphrase is returned when an empty passphrase is provided.
+var ErrEmptyPassphrase = errors.New("passphrase cannot be empty")
+
 // Encrypt encrypts data using age with a passphrase (scrypt mode).
 // The passphrase is used to derive an encryption key using scrypt.
 func Encrypt(dst io.Writer, src io.Reader, passphrase string) error {
+	if passphrase == "" {
+		return ErrEmptyPassphrase
+	}
 	recipient, err := age.NewScryptRecipient(passphrase)
 	if err != nil {
 		return fmt.Errorf("creating recipient: %w", err)
@@ -33,6 +40,9 @@ func Encrypt(dst io.Writer, src io.Reader, passphrase string) error {
 
 // Decrypt decrypts age-encrypted data using a passphrase.
 func Decrypt(dst io.Writer, src io.Reader, passphrase string) error {
+	if passphrase == "" {
+		return ErrEmptyPassphrase
+	}
 	identity, err := age.NewScryptIdentity(passphrase)
 	if err != nil {
 		return fmt.Errorf("creating identity: %w", err)

internal/crypto/hash.go

@@ -2,6 +2,7 @@ package crypto
 
 import (
 	"crypto/sha256"
+	"crypto/subtle"
 	"encoding/hex"
 	"fmt"
 	"io"
@@ -37,6 +38,7 @@ func HashFile(path string) (string, error) {
 }
 
 // VerifyHash checks if the given hash matches the expected value.
+// Uses constant-time comparison to prevent timing attacks.
 func VerifyHash(got, expected string) bool {
-	return got == expected
+	return subtle.ConstantTimeCompare([]byte(got), []byte(expected)) == 1
 }

internal/html/assets/app.js

@@ -71,7 +71,7 @@
           elements.loadingOverlay.classList.add('hidden');
           return;
         } catch (e) {
-          console.error('Embedded WASM failed:', e);
+          // WASM initialization failed - will show error to user below
         }
       }
       showError(t('error', err.message));
@@ -146,7 +146,7 @@
           await parseAndAddShare(content, file.name);
         }
       } catch (err) {
-        console.error('Error reading file:', err);
+        showError(t('error', 'Failed to read file'));
       }
     }
   }
@@ -424,6 +424,16 @@
     a.download = 'manifest.tar.gz';
     a.click();
     URL.revokeObjectURL(url);
+
+    // Security: Clear sensitive data from memory after download
+    clearSensitiveState();
+  }
+
+  // Clear sensitive data from state to minimize memory exposure
+  function clearSensitiveState() {
+    state.decryptedArchive = null;
+    state.manifest = null;
+    // Note: shares contain metadata but not the actual secret
   }
 
   // Utility functions
@@ -459,7 +469,7 @@
 
   function showError(msg) {
     alert(msg); // Simple for now, could be a toast
-    console.error(msg);
+    // Note: Removed console.error to avoid logging sensitive information
   }
 
   // Start

internal/manifest/archive.go

@@ -10,6 +10,13 @@ import (
 	"strings"
 )
 
+const (
+	// MaxFileSize is the maximum size of a single file during extraction (100 MB).
+	MaxFileSize = 100 * 1024 * 1024
+	// MaxTotalSize is the maximum total size of all extracted files (1 GB).
+	MaxTotalSize = 1024 * 1024 * 1024
+)
+
 // Archive creates a tar.gz archive of the given directory.
 // The archive preserves the directory structure relative to the source.
 func Archive(w io.Writer, sourceDir string) error {
@@ -104,6 +111,7 @@ func Extract(r io.Reader, destDir string) (string, error) {
 
 	tr := tar.NewReader(gzr)
 	var rootDir string
+	var totalSize int64
 
 	for {
 		header, err := tr.Next()
@@ -134,6 +142,15 @@ func Extract(r io.Reader, destDir string) (string, error) {
 			}
 
 		case tar.TypeReg:
+			// Security: enforce file size limit
+			if header.Size > MaxFileSize {
+				return "", fmt.Errorf("file exceeds maximum size of %d bytes", MaxFileSize)
+			}
+			totalSize += header.Size
+			if totalSize > MaxTotalSize {
+				return "", fmt.Errorf("archive exceeds maximum total size of %d bytes", MaxTotalSize)
+			}
+
 			if err := os.MkdirAll(filepath.Dir(target), 0755); err != nil {
 				return "", fmt.Errorf("creating parent directory: %w", err)
 			}
@@ -143,11 +160,16 @@ func Extract(r io.Reader, destDir string) (string, error) {
 				return "", fmt.Errorf("creating file %s: %w", target, err)
 			}
 
-			if _, err := io.Copy(f, tr); err != nil {
-				f.Close()
+			// Use LimitReader to enforce size limit during actual copy
+			limitedReader := io.LimitReader(tr, MaxFileSize+1)
+			written, err := io.Copy(f, limitedReader)
+			f.Close()
+			if err != nil {
 				return "", fmt.Errorf("writing file %s: %w", target, err)
 			}
-			f.Close()
+			if written > MaxFileSize {
+				return "", fmt.Errorf("file exceeds maximum size during extraction")
+			}
 
 		default:
 			// Skip other types (symlinks, etc.) for security

internal/shamir/share.go

@@ -169,10 +169,11 @@ func ParseShare(content []byte) (*Share, error) {
 }
 
 // Verify checks that the share's checksum matches its data.
+// Uses constant-time comparison to prevent timing attacks.
 func (s *Share) Verify() error {
 	computed := crypto.HashBytes(s.Data)
-	if computed != s.Checksum {
-		return fmt.Errorf("checksum mismatch: expected %s, got %s", s.Checksum, computed)
+	if !crypto.VerifyHash(computed, s.Checksum) {
+		return fmt.Errorf("share checksum verification failed")
 	}
 	return nil
 }

internal/wasm/recover.go

@@ -46,6 +46,11 @@ type ExtractedFile struct {
 const (
 	shareBegin = "-----BEGIN REMEMORY SHARE-----"
 	shareEnd   = "-----END REMEMORY SHARE-----"
+
+	// MaxFileSize is the maximum size of a single file during extraction (100 MB).
+	MaxFileSize = 100 * 1024 * 1024
+	// MaxTotalSize is the maximum total size of all extracted files (1 GB).
+	MaxTotalSize = 1024 * 1024 * 1024
 )
 
 // parseShare extracts a share from text content (which might be a full README.txt).
@@ -206,6 +211,7 @@ func extractTarGz(tarGzData []byte) ([]ExtractedFile, error) {
 
 	tr := tar.NewReader(gzr)
 	var files []ExtractedFile
+	var totalSize int64
 
 	// Regex to detect path traversal
 	pathTraversal := regexp.MustCompile(`(^|/)\.\.(/|$)`)
@@ -221,17 +227,28 @@ func extractTarGz(tarGzData []byte) ([]ExtractedFile, error) {
 
 		// Security: reject path traversal
 		if pathTraversal.MatchString(header.Name) {
-			return nil, fmt.Errorf("invalid path in archive: %s", header.Name)
+			return nil, fmt.Errorf("archive contains invalid path")
 		}
 
 		// Skip directories, only extract regular files
 		if header.Typeflag != tar.TypeReg {
 			continue
 		}
 
-		data, err := io.ReadAll(tr)
+		// Security: enforce file size limits
+		if header.Size > MaxFileSize {
+			return nil, fmt.Errorf("file exceeds maximum allowed size")
+		}
+		totalSize += header.Size
+		if totalSize > MaxTotalSize {
+			return nil, fmt.Errorf("archive exceeds maximum total size")
+		}
+
+		// Use LimitReader for additional safety
+		limitedReader := io.LimitReader(tr, MaxFileSize)
+		data, err := io.ReadAll(limitedReader)
 		if err != nil {
-			return nil, fmt.Errorf("reading file %s: %w", header.Name, err)
+			return nil, fmt.Errorf("failed to read file from archive")
 		}
 
 		files = append(files, ExtractedFile{