From 33cc882d98190af9fbf5446894b6821249506c74 Mon Sep 17 00:00:00 2001 From: Gusted Date: Sat, 31 Dec 2022 01:02:00 +0100 Subject: [PATCH 1/5] Unify hashing for avatar - Unify the hashing code for repo and user avatars into a function. - Use a sane hash function instead of MD5. - Only require hashing instead of twice(w.r.t. hashing for user avatar). - Improve the comment for the hashing code of why it works. --- modules/avatar/hash.go | 26 ++++++++++++++++++++++++++ services/repository/avatar.go | 3 +-- services/user/user.go | 7 +------ 3 files changed, 28 insertions(+), 8 deletions(-) create mode 100644 modules/avatar/hash.go diff --git a/modules/avatar/hash.go b/modules/avatar/hash.go new file mode 100644 index 0000000000000..7233e20b362b6 --- /dev/null +++ b/modules/avatar/hash.go @@ -0,0 +1,26 @@ +package avatar + +import ( + "crypto/sha256" + "encoding/hex" + "fmt" + "strconv" +) + +// HashAvatar will generate a unique string, which ensures that when there's a +// different unique ID while the data is the same, it will generate a different +// output. It will generate the output according to: +// HEX(HASH(uniqueID || - || data)) +// The hash is being used is SHA256. +// The sole purpose of the unique ID is to generate a distinct hash Such that +// two unique IDs with the same data will have a different hash output. +// The "-" byte is important to ensure that data cannot be modified such that +// the first byte is a number, which could lead to a "collision" with the hash +// of another unique ID. +func HashAvatar(uniqueID int64, data []byte) string { + h := sha256.New() + h.Write([]byte(strconv.FormatInt(uniqueID, 10))) + h.Write([]byte{'-'}) + h.Write(data) + return hex.EncodeToString(h.Sum(nil)) +} diff --git a/services/repository/avatar.go b/services/repository/avatar.go index a829a1000ae3d..5fe8bd2c72f88 100644 --- a/services/repository/avatar.go +++ b/services/repository/avatar.go @@ -5,7 +5,6 @@ package repository import ( "context" - "crypto/md5" "fmt" "image/png" "io" @@ -27,7 +26,7 @@ func UploadAvatar(repo *repo_model.Repository, data []byte) error { return err } - newAvatar := fmt.Sprintf("%d-%x", repo.ID, md5.Sum(data)) + newAvatar := avatar.HashAvatar(repo.ID, data) if repo.Avatar == newAvatar { // upload the same picture return nil } diff --git a/services/user/user.go b/services/user/user.go index 65db732bf917a..c95eb67a851e9 100644 --- a/services/user/user.go +++ b/services/user/user.go @@ -5,7 +5,6 @@ package user import ( "context" - "crypto/md5" "fmt" "image/png" "io" @@ -241,11 +240,7 @@ func UploadAvatar(u *user_model.User, data []byte) error { defer committer.Close() u.UseCustomAvatar = true - // Different users can upload same image as avatar - // If we prefix it with u.ID, it will be separated - // Otherwise, if any of the users delete his avatar - // Other users will lose their avatars too. - u.Avatar = fmt.Sprintf("%x", md5.Sum([]byte(fmt.Sprintf("%d-%x", u.ID, md5.Sum(data))))) + u.Avatar = avatar.HashAvatar(u.ID, data) if err = user_model.UpdateUserCols(ctx, u, "use_custom_avatar", "avatar"); err != nil { return fmt.Errorf("updateUser: %w", err) } From 77a31abf9ef404f8bff487f02f2ff3e79a0fe94f Mon Sep 17 00:00:00 2001 From: Gusted Date: Sat, 31 Dec 2022 01:12:30 +0100 Subject: [PATCH 2/5] Fix typo --- modules/avatar/hash.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/avatar/hash.go b/modules/avatar/hash.go index 7233e20b362b6..d0fa4dc314413 100644 --- a/modules/avatar/hash.go +++ b/modules/avatar/hash.go @@ -11,7 +11,7 @@ import ( // different unique ID while the data is the same, it will generate a different // output. It will generate the output according to: // HEX(HASH(uniqueID || - || data)) -// The hash is being used is SHA256. +// The hash being used is SHA256. // The sole purpose of the unique ID is to generate a distinct hash Such that // two unique IDs with the same data will have a different hash output. // The "-" byte is important to ensure that data cannot be modified such that From cfb6b3c13ddeabe452550194e6bcb41b92e1104b Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Sun, 1 Jan 2023 22:10:22 +0800 Subject: [PATCH 3/5] Update modules/avatar/hash.go Co-authored-by: Yarden Shoham --- modules/avatar/hash.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/modules/avatar/hash.go b/modules/avatar/hash.go index d0fa4dc314413..76927be7ae6f8 100644 --- a/modules/avatar/hash.go +++ b/modules/avatar/hash.go @@ -1,3 +1,6 @@ +// Copyright 2023 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + package avatar import ( From 1974510db9b5bf03c7b4f741c1e07bc32c33f306 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Mon, 2 Jan 2023 00:16:50 +0800 Subject: [PATCH 4/5] Update modules/avatar/hash.go Co-authored-by: Yarden Shoham --- modules/avatar/hash.go | 1 - 1 file changed, 1 deletion(-) diff --git a/modules/avatar/hash.go b/modules/avatar/hash.go index 76927be7ae6f8..50db9c1943a30 100644 --- a/modules/avatar/hash.go +++ b/modules/avatar/hash.go @@ -6,7 +6,6 @@ package avatar import ( "crypto/sha256" "encoding/hex" - "fmt" "strconv" ) From 6109806d175e4a04943ce8a587161e533275ff20 Mon Sep 17 00:00:00 2001 From: Gusted Date: Mon, 2 Jan 2023 15:37:01 +0100 Subject: [PATCH 5/5] Fix test --- services/repository/avatar_test.go | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/services/repository/avatar_test.go b/services/repository/avatar_test.go index 3875302696d03..5ec899ec3f9a7 100644 --- a/services/repository/avatar_test.go +++ b/services/repository/avatar_test.go @@ -5,14 +5,13 @@ package repository import ( "bytes" - "crypto/md5" - "fmt" "image" "image/png" "testing" repo_model "code.gitea.io/gitea/models/repo" "code.gitea.io/gitea/models/unittest" + "code.gitea.io/gitea/modules/avatar" "github.com/stretchr/testify/assert" ) @@ -28,7 +27,7 @@ func TestUploadAvatar(t *testing.T) { err := UploadAvatar(repo, buff.Bytes()) assert.NoError(t, err) - assert.Equal(t, fmt.Sprintf("%d-%x", 10, md5.Sum(buff.Bytes())), repo.Avatar) + assert.Equal(t, avatar.HashAvatar(10, buff.Bytes()), repo.Avatar) } func TestUploadBigAvatar(t *testing.T) {