From 7ea637be10cbb92839d42749cd3ff42a584d176c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jes=C3=BAs=20Espino?= Date: Thu, 16 Apr 2020 15:23:27 +0200 Subject: [PATCH] MM-20934: Fixing int overflow in 32 bits on MaxImageSize check (#14280) * MM-20934: Fixing int overflow in 32 bits on MaxImageSize check * Adding comments explaining the casting and the bug fixed there * Apply suggestions from code review Co-Authored-By: Juho Nurminen * Fixing store layers Co-authored-by: Juho Nurminen --- app/brand.go | 5 ++++- app/file.go | 12 +++++++++--- app/team.go | 6 +++++- app/user.go | 5 ++++- model/file.go | 2 +- 5 files changed, 23 insertions(+), 7 deletions(-) diff --git a/app/brand.go b/app/brand.go index 1a8d5795fe..7710c0647f 100644 --- a/app/brand.go +++ b/app/brand.go @@ -38,7 +38,10 @@ func (a *App) SaveBrandImage(imageData *multipart.FileHeader) *model.AppError { return model.NewAppError("SaveBrandImage", "brand.save_brand_image.decode_config.app_error", nil, err.Error(), http.StatusBadRequest) } - if config.Width*config.Height > model.MaxImageSize { + // This casting is done to prevent overflow on 32 bit systems (not needed + // in 64 bits systems because images can't have more than 32 bits height or + // width) + if int64(config.Width)*int64(config.Height) > model.MaxImageSize { return model.NewAppError("SaveBrandImage", "brand.save_brand_image.too_large.app_error", nil, "", http.StatusBadRequest) } diff --git a/app/file.go b/app/file.go index 40b9ee650c..e9978cd598 100644 --- a/app/file.go +++ b/app/file.go @@ -55,7 +55,7 @@ const ( RotatedCCWMirrored = 7 RotatedCW = 8 - MaxImageSize = 6048 * 4032 // 24 megapixels, roughly 36MB as a raw image + MaxImageSize = int64(6048 * 4032) // 24 megapixels, roughly 36MB as a raw image ImageThumbnailWidth = 120 ImageThumbnailHeight = 100 ImageThumbnailRatio = float64(ImageThumbnailHeight) / float64(ImageThumbnailWidth) @@ -728,7 +728,10 @@ func (t *UploadFileTask) preprocessImage() *model.AppError { t.fileinfo.Height = config.Height // Check dimensions before loading the whole thing into memory later on. - if t.fileinfo.Width*t.fileinfo.Height > MaxImageSize { + // This casting is done to prevent overflow on 32 bit systems (not needed + // in 64 bits systems because images can't have more than 32 bits height or + // width) + if int64(t.fileinfo.Width)*int64(t.fileinfo.Height) > MaxImageSize { return t.newAppError("api.file.upload_file.large_image_detailed.app_error", "", http.StatusBadRequest) } @@ -911,7 +914,10 @@ func (a *App) DoUploadFileExpectModification(now time.Time, rawTeamId string, ra if info.IsImage() { // Check dimensions before loading the whole thing into memory later on - if info.Width*info.Height > MaxImageSize { + // This casting is done to prevent overflow on 32 bit systems (not needed + // in 64 bits systems because images can't have more than 32 bits height or + // width) + if int64(info.Width)*int64(info.Height) > MaxImageSize { err := model.NewAppError("uploadFile", "api.file.upload_file.large_image.app_error", map[string]interface{}{"Filename": filename}, "", http.StatusBadRequest) return nil, data, err } diff --git a/app/team.go b/app/team.go index ece759672d..8990532fbd 100644 --- a/app/team.go +++ b/app/team.go @@ -1510,7 +1510,11 @@ func (a *App) SetTeamIconFromMultiPartFile(teamId string, file multipart.File) * if err != nil { return model.NewAppError("SetTeamIcon", "api.team.set_team_icon.decode_config.app_error", nil, err.Error(), http.StatusBadRequest) } - if config.Width*config.Height > model.MaxImageSize { + + // This casting is done to prevent overflow on 32 bit systems (not needed + // in 64 bits systems because images can't have more than 32 bits height or + // width) + if int64(config.Width)*int64(config.Height) > model.MaxImageSize { return model.NewAppError("SetTeamIcon", "api.team.set_team_icon.too_large.app_error", nil, "", http.StatusBadRequest) } diff --git a/app/user.go b/app/user.go index d227b7cbf2..c84df4f4c6 100644 --- a/app/user.go +++ b/app/user.go @@ -833,7 +833,10 @@ func (a *App) SetProfileImageFromMultiPartFile(userId string, file multipart.Fil if err != nil { return model.NewAppError("SetProfileImage", "api.user.upload_profile_user.decode_config.app_error", nil, err.Error(), http.StatusBadRequest) } - if config.Width*config.Height > model.MaxImageSize { + // This casting is done to prevent overflow on 32 bit systems (not needed + // in 64 bits systems because images can't have more than 32 bits height or + // width) + if int64(config.Width)*int64(config.Height) > model.MaxImageSize { return model.NewAppError("SetProfileImage", "api.user.upload_profile_user.too_large.app_error", nil, "", http.StatusBadRequest) } diff --git a/model/file.go b/model/file.go index 45b15d59b5..9f76bac174 100644 --- a/model/file.go +++ b/model/file.go @@ -9,7 +9,7 @@ import ( ) const ( - MaxImageSize = 6048 * 4032 // 24 megapixels, roughly 36MB as a raw image + MaxImageSize = int64(6048 * 4032) // 24 megapixels, roughly 36MB as a raw image ) var (