feat(security): use bcrypt hashes and safe paths for boxes
- Replace legacy salted password hashing with bcrypt and store hash alg - Accept existing bcrypt hashes while keeping legacy verification fallback - Validate box IDs and use SafeChildPath for box/file operations to prevent traversal - Refactor download flow to share zip writer logic and correctly handle one-time deletes and optional renew-on-download only after a successful zip writefeat(security): use bcrypt hashes and safe paths for boxes - Replace legacy salted password hashing with bcrypt and store hash alg - Accept existing bcrypt hashes while keeping legacy verification fallback - Validate box IDs and use SafeChildPath for box/file operations to prevent traversal - Refactor download flow to share zip writer logic and correctly handle one-time deletes and optional renew-on-download only after a successful zip write
This commit is contained in:
@@ -18,6 +18,8 @@ import (
|
||||
"sync"
|
||||
"time"
|
||||
|
||||
"golang.org/x/crypto/bcrypt"
|
||||
|
||||
"warpbox/lib/helpers"
|
||||
"warpbox/lib/models"
|
||||
)
|
||||
@@ -76,16 +78,39 @@ func BoxPath(boxID string) string {
|
||||
return filepath.Join(uploadRoot, boxID)
|
||||
}
|
||||
|
||||
func safeBoxPath(boxID string) (string, bool) {
|
||||
if !ValidBoxID(boxID) {
|
||||
return "", false
|
||||
}
|
||||
return helpers.SafeChildPath(uploadRoot, boxID)
|
||||
}
|
||||
|
||||
func ManifestPath(boxID string) string {
|
||||
return filepath.Join(BoxPath(boxID), manifestFile)
|
||||
}
|
||||
|
||||
func SafeBoxFilePath(boxID string, filename string) (string, bool) {
|
||||
return helpers.SafeChildPath(BoxPath(boxID), filename)
|
||||
boxPath, ok := safeBoxPath(boxID)
|
||||
if !ok {
|
||||
return "", false
|
||||
}
|
||||
return helpers.SafeChildPath(boxPath, filename)
|
||||
}
|
||||
|
||||
func IsSafeRegularBoxFile(boxID string, filename string) bool {
|
||||
path, ok := SafeBoxFilePath(boxID, filename)
|
||||
if !ok {
|
||||
return false
|
||||
}
|
||||
return ensureRegularFile(path) == nil
|
||||
}
|
||||
|
||||
func DeleteBox(boxID string) error {
|
||||
return os.RemoveAll(BoxPath(boxID))
|
||||
boxPath, ok := safeBoxPath(boxID)
|
||||
if !ok {
|
||||
return fmt.Errorf("Invalid box id")
|
||||
}
|
||||
return os.RemoveAll(boxPath)
|
||||
}
|
||||
|
||||
func ListBoxSummaries() ([]models.BoxSummary, error) {
|
||||
@@ -218,18 +243,17 @@ func CreateManifest(boxID string, request models.CreateBoxRequest) ([]models.Box
|
||||
}
|
||||
|
||||
if password := strings.TrimSpace(request.Password); password != "" {
|
||||
salt, err := helpers.RandomHexID(16)
|
||||
if err != nil {
|
||||
return nil, fmt.Errorf("Could not secure upload box")
|
||||
}
|
||||
|
||||
authToken, err := helpers.RandomHexID(16)
|
||||
if err != nil {
|
||||
return nil, fmt.Errorf("Could not secure upload box")
|
||||
}
|
||||
passwordHash, err := bcrypt.GenerateFromPassword([]byte(password), bcrypt.DefaultCost)
|
||||
if err != nil {
|
||||
return nil, fmt.Errorf("Could not secure upload box")
|
||||
}
|
||||
|
||||
manifest.PasswordSalt = salt
|
||||
manifest.PasswordHash = passwordHash(salt, password)
|
||||
manifest.PasswordHash = string(passwordHash)
|
||||
manifest.PasswordHashAlg = "bcrypt"
|
||||
manifest.AuthToken = authToken
|
||||
}
|
||||
|
||||
@@ -250,7 +274,7 @@ func IsExpired(manifest models.BoxManifest) bool {
|
||||
}
|
||||
|
||||
func IsPasswordProtected(manifest models.BoxManifest) bool {
|
||||
return manifest.PasswordSalt != "" && manifest.PasswordHash != "" && manifest.AuthToken != ""
|
||||
return manifest.PasswordHash != "" && manifest.AuthToken != ""
|
||||
}
|
||||
|
||||
func VerifyPassword(manifest models.BoxManifest, password string) bool {
|
||||
@@ -259,7 +283,11 @@ func VerifyPassword(manifest models.BoxManifest, password string) bool {
|
||||
}
|
||||
|
||||
expected := manifest.PasswordHash
|
||||
actual := passwordHash(manifest.PasswordSalt, password)
|
||||
if manifest.PasswordHashAlg == "bcrypt" || strings.HasPrefix(expected, "$2") {
|
||||
return bcrypt.CompareHashAndPassword([]byte(expected), []byte(password)) == nil
|
||||
}
|
||||
|
||||
actual := legacyPasswordHash(manifest.PasswordSalt, password)
|
||||
return subtle.ConstantTimeCompare([]byte(expected), []byte(actual)) == 1
|
||||
}
|
||||
|
||||
@@ -335,13 +363,25 @@ func RenewManifest(boxID string, seconds int64) (models.BoxManifest, error) {
|
||||
}
|
||||
|
||||
func AddFileToZip(zipWriter *zip.Writer, boxID string, filename string) error {
|
||||
source, err := os.Open(filepath.Join(BoxPath(boxID), filename))
|
||||
path, ok := SafeBoxFilePath(boxID, filename)
|
||||
if !ok {
|
||||
return fmt.Errorf("Invalid file")
|
||||
}
|
||||
if err := ensureRegularFile(path); err != nil {
|
||||
return err
|
||||
}
|
||||
zipName, ok := safeZipEntryName(filename)
|
||||
if !ok {
|
||||
return fmt.Errorf("Invalid zip entry")
|
||||
}
|
||||
|
||||
source, err := os.Open(path)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
defer source.Close()
|
||||
|
||||
destination, err := zipWriter.Create(filename)
|
||||
destination, err := zipWriter.Create(zipName)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
@@ -358,6 +398,9 @@ func SaveManifestUpload(boxID string, fileID string, file *multipart.FileHeader)
|
||||
if err != nil {
|
||||
return models.BoxFile{}, err
|
||||
}
|
||||
if IsExpired(manifest) {
|
||||
return models.BoxFile{}, fmt.Errorf("Box expired")
|
||||
}
|
||||
|
||||
fileIndex := -1
|
||||
for index, manifestFile := range manifest.Files {
|
||||
@@ -376,7 +419,10 @@ func SaveManifestUpload(boxID string, fileID string, file *multipart.FileHeader)
|
||||
return models.BoxFile{}, fmt.Errorf("Could not prepare upload box")
|
||||
}
|
||||
|
||||
destination := filepath.Join(BoxPath(boxID), filename)
|
||||
destination, ok := SafeBoxFilePath(boxID, filename)
|
||||
if !ok {
|
||||
return models.BoxFile{}, fmt.Errorf("Invalid filename")
|
||||
}
|
||||
if err := saveMultipartFile(file, destination); err != nil {
|
||||
manifest.Files[fileIndex].Status = models.FileStatusFailed
|
||||
startRetentionIfTerminalUnlocked(&manifest)
|
||||
@@ -407,7 +453,10 @@ func SaveUpload(boxID string, file *multipart.FileHeader) (models.BoxFile, error
|
||||
}
|
||||
|
||||
filename = helpers.UniqueFilename(boxPath, filename)
|
||||
destination := filepath.Join(boxPath, filename)
|
||||
destination, ok := SafeBoxFilePath(boxID, filename)
|
||||
if !ok {
|
||||
return models.BoxFile{}, fmt.Errorf("Invalid filename")
|
||||
}
|
||||
if err := saveMultipartFile(file, destination); err != nil {
|
||||
return models.BoxFile{}, fmt.Errorf("Could not save uploaded file")
|
||||
}
|
||||
@@ -423,7 +472,9 @@ func SaveUpload(boxID string, file *multipart.FileHeader) (models.BoxFile, error
|
||||
|
||||
func DecorateFile(boxID string, file models.BoxFile) models.BoxFile {
|
||||
if file.MimeType == "" {
|
||||
file.MimeType = helpers.MimeTypeForFile(filepath.Join(BoxPath(boxID), file.Name), file.Name)
|
||||
if path, ok := SafeBoxFilePath(boxID, file.Name); ok {
|
||||
file.MimeType = helpers.MimeTypeForFile(path, file.Name)
|
||||
}
|
||||
}
|
||||
|
||||
if file.SizeLabel == "" {
|
||||
@@ -495,9 +546,12 @@ func reconcileManifest(boxID string) (models.BoxManifest, error) {
|
||||
|
||||
changed := false
|
||||
for index, file := range manifest.Files {
|
||||
path := filepath.Join(BoxPath(boxID), file.Name)
|
||||
path, ok := SafeBoxFilePath(boxID, file.Name)
|
||||
if !ok || ensureRegularFile(path) != nil {
|
||||
continue
|
||||
}
|
||||
info, err := os.Stat(path)
|
||||
if err != nil {
|
||||
if err != nil || !info.Mode().IsRegular() {
|
||||
continue
|
||||
}
|
||||
|
||||
@@ -531,7 +585,7 @@ func listCompletedFilesFromDisk(boxID string) ([]models.BoxFile, error) {
|
||||
|
||||
files := make([]models.BoxFile, 0, len(entries))
|
||||
for _, entry := range entries {
|
||||
if entry.IsDir() || entry.Name() == manifestFile {
|
||||
if entry.IsDir() || entry.Name() == manifestFile || entry.Type()&os.ModeSymlink != 0 {
|
||||
continue
|
||||
}
|
||||
|
||||
@@ -539,6 +593,9 @@ func listCompletedFilesFromDisk(boxID string) ([]models.BoxFile, error) {
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
if !info.Mode().IsRegular() {
|
||||
continue
|
||||
}
|
||||
|
||||
name := entry.Name()
|
||||
files = append(files, DecorateFile(boxID, models.BoxFile{
|
||||
@@ -601,7 +658,7 @@ func startRetentionIfTerminalUnlocked(manifest *models.BoxManifest) {
|
||||
manifest.ExpiresAt = time.Now().UTC().Add(time.Duration(seconds) * time.Second)
|
||||
}
|
||||
|
||||
func passwordHash(salt string, password string) string {
|
||||
func legacyPasswordHash(salt string, password string) string {
|
||||
sum := sha256.Sum256([]byte(salt + ":" + password))
|
||||
return hex.EncodeToString(sum[:])
|
||||
}
|
||||
@@ -624,12 +681,64 @@ func saveMultipartFile(file *multipart.FileHeader, destination string) error {
|
||||
}
|
||||
defer source.Close()
|
||||
|
||||
target, err := os.Create(destination)
|
||||
target, tempPath, err := createTempSibling(destination)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
defer target.Close()
|
||||
committed := false
|
||||
defer func() {
|
||||
target.Close()
|
||||
if !committed {
|
||||
os.Remove(tempPath)
|
||||
}
|
||||
}()
|
||||
|
||||
_, err = io.Copy(target, source)
|
||||
return err
|
||||
if _, err := io.Copy(target, source); err != nil {
|
||||
return err
|
||||
}
|
||||
if err := target.Close(); err != nil {
|
||||
return err
|
||||
}
|
||||
if err := os.Rename(tempPath, destination); err != nil {
|
||||
return err
|
||||
}
|
||||
committed = true
|
||||
return nil
|
||||
}
|
||||
|
||||
func createTempSibling(destination string) (*os.File, string, error) {
|
||||
directory := filepath.Dir(destination)
|
||||
if err := os.MkdirAll(directory, 0755); err != nil {
|
||||
return nil, "", err
|
||||
}
|
||||
|
||||
target, err := os.CreateTemp(directory, ".warpbox-upload-*")
|
||||
if err != nil {
|
||||
return nil, "", err
|
||||
}
|
||||
return target, target.Name(), nil
|
||||
}
|
||||
|
||||
func safeZipEntryName(filename string) (string, bool) {
|
||||
filename = strings.TrimSpace(filename)
|
||||
if filename == "" || filepath.IsAbs(filename) {
|
||||
return "", false
|
||||
}
|
||||
|
||||
cleaned := filepath.ToSlash(filepath.Clean(filename))
|
||||
if cleaned == "." || cleaned == ".." || strings.HasPrefix(cleaned, "../") || strings.HasPrefix(cleaned, "/") {
|
||||
return "", false
|
||||
}
|
||||
return cleaned, true
|
||||
}
|
||||
|
||||
func ensureRegularFile(path string) error {
|
||||
info, err := os.Lstat(path)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
if info.Mode()&os.ModeSymlink != 0 || !info.Mode().IsRegular() {
|
||||
return fmt.Errorf("Invalid file")
|
||||
}
|
||||
return nil
|
||||
}
|
||||
|
||||
@@ -1,6 +1,10 @@
|
||||
package boxstore
|
||||
|
||||
import (
|
||||
"archive/zip"
|
||||
"bytes"
|
||||
"os"
|
||||
"path/filepath"
|
||||
"testing"
|
||||
"time"
|
||||
|
||||
@@ -59,3 +63,87 @@ func TestStartRetentionSkipsOneTimeDownload(t *testing.T) {
|
||||
t.Fatalf("expected one-time download box to avoid retention expiry, got %s", manifest.ExpiresAt)
|
||||
}
|
||||
}
|
||||
|
||||
func TestSafeBoxFilePathRejectsTraversal(t *testing.T) {
|
||||
restoreUploadRoot := UploadRoot()
|
||||
defer SetUploadRoot(restoreUploadRoot)
|
||||
SetUploadRoot(t.TempDir())
|
||||
|
||||
boxID := "0123456789abcdef0123456789abcdef"
|
||||
if _, ok := SafeBoxFilePath(boxID, "../outside.txt"); ok {
|
||||
t.Fatal("expected traversal to be rejected")
|
||||
}
|
||||
if _, ok := SafeBoxFilePath("../bad", "file.txt"); ok {
|
||||
t.Fatal("expected invalid box id to be rejected")
|
||||
}
|
||||
}
|
||||
|
||||
func TestAddFileToZipRejectsUnsafeManifestName(t *testing.T) {
|
||||
restoreUploadRoot := UploadRoot()
|
||||
defer SetUploadRoot(restoreUploadRoot)
|
||||
SetUploadRoot(t.TempDir())
|
||||
|
||||
var buffer bytes.Buffer
|
||||
zipWriter := zip.NewWriter(&buffer)
|
||||
if err := AddFileToZip(zipWriter, "0123456789abcdef0123456789abcdef", "../outside.txt"); err == nil {
|
||||
t.Fatal("expected unsafe zip filename to be rejected")
|
||||
}
|
||||
}
|
||||
|
||||
func TestListFilesSkipsSymlinks(t *testing.T) {
|
||||
restoreUploadRoot := UploadRoot()
|
||||
defer SetUploadRoot(restoreUploadRoot)
|
||||
SetUploadRoot(t.TempDir())
|
||||
|
||||
boxID := "0123456789abcdef0123456789abcdef"
|
||||
if err := os.MkdirAll(BoxPath(boxID), 0755); err != nil {
|
||||
t.Fatalf("MkdirAll returned error: %v", err)
|
||||
}
|
||||
if err := os.WriteFile(filepath.Join(BoxPath(boxID), "safe.txt"), []byte("safe"), 0644); err != nil {
|
||||
t.Fatalf("WriteFile returned error: %v", err)
|
||||
}
|
||||
if err := os.Symlink(filepath.Join(BoxPath(boxID), "safe.txt"), filepath.Join(BoxPath(boxID), "link.txt")); err != nil {
|
||||
t.Skipf("symlink unavailable: %v", err)
|
||||
}
|
||||
|
||||
files, err := ListFiles(boxID)
|
||||
if err != nil {
|
||||
t.Fatalf("ListFiles returned error: %v", err)
|
||||
}
|
||||
if len(files) != 1 || files[0].Name != "safe.txt" {
|
||||
t.Fatalf("expected only regular file, got %#v", files)
|
||||
}
|
||||
}
|
||||
|
||||
func TestBoxPasswordUsesBcryptAndVerifiesLegacy(t *testing.T) {
|
||||
restoreUploadRoot := UploadRoot()
|
||||
defer SetUploadRoot(restoreUploadRoot)
|
||||
SetUploadRoot(t.TempDir())
|
||||
|
||||
boxID := "0123456789abcdef0123456789abcdef"
|
||||
if err := os.MkdirAll(BoxPath(boxID), 0755); err != nil {
|
||||
t.Fatalf("MkdirAll returned error: %v", err)
|
||||
}
|
||||
if _, err := CreateManifest(boxID, models.CreateBoxRequest{Password: "secret"}); err != nil {
|
||||
t.Fatalf("CreateManifest returned error: %v", err)
|
||||
}
|
||||
manifest, err := ReadManifest(boxID)
|
||||
if err != nil {
|
||||
t.Fatalf("ReadManifest returned error: %v", err)
|
||||
}
|
||||
if manifest.PasswordHashAlg != "bcrypt" {
|
||||
t.Fatalf("expected bcrypt password hash, got %q", manifest.PasswordHashAlg)
|
||||
}
|
||||
if !VerifyPassword(manifest, "secret") {
|
||||
t.Fatal("expected bcrypt password to verify")
|
||||
}
|
||||
|
||||
legacy := models.BoxManifest{
|
||||
PasswordSalt: "salt",
|
||||
PasswordHash: legacyPasswordHash("salt", "secret"),
|
||||
AuthToken: "token",
|
||||
}
|
||||
if !VerifyPassword(legacy, "secret") {
|
||||
t.Fatal("expected legacy password hash to verify")
|
||||
}
|
||||
}
|
||||
|
||||
@@ -140,7 +140,15 @@ func canGenerateThumbnail(file models.BoxFile) bool {
|
||||
}
|
||||
|
||||
func generateThumbnail(task thumbnailTask) error {
|
||||
source, err := os.Open(filepath.Join(BoxPath(task.BoxID), task.Name))
|
||||
sourcePath, ok := SafeBoxFilePath(task.BoxID, task.Name)
|
||||
if !ok {
|
||||
return os.ErrInvalid
|
||||
}
|
||||
if err := ensureRegularFile(sourcePath); err != nil {
|
||||
return err
|
||||
}
|
||||
|
||||
source, err := os.Open(sourcePath)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
@@ -161,15 +169,28 @@ func generateThumbnail(task thumbnailTask) error {
|
||||
return os.ErrInvalid
|
||||
}
|
||||
|
||||
target, err := os.Create(path)
|
||||
target, tempPath, err := createTempSibling(path)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
defer target.Close()
|
||||
committed := false
|
||||
defer func() {
|
||||
target.Close()
|
||||
if !committed {
|
||||
os.Remove(tempPath)
|
||||
}
|
||||
}()
|
||||
|
||||
if err := jpeg.Encode(target, thumb, &jpeg.Options{Quality: 82}); err != nil {
|
||||
return err
|
||||
}
|
||||
if err := target.Close(); err != nil {
|
||||
return err
|
||||
}
|
||||
if err := os.Rename(tempPath, path); err != nil {
|
||||
return err
|
||||
}
|
||||
committed = true
|
||||
|
||||
return markThumbnailReady(task.BoxID, task.FileID)
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user