Local storage should not store files as executable (#22162) (#22163)

zeripath · web-flow · commit 56bded9d8d1b · 2022-12-19T01:12:25.000+02:00
Backport #22162 The PR #21198 introduced a probable security vulnerability which resulted in making all storage files be marked as executable. This PR ensures that these are forcibly marked as non-executable. Fix #22161 Signed-off-by: Andrew Thornton <art27@cantab.net> Signed-off-by: Andrew Thornton <art27@cantab.net>
diff --git a/modules/storage/local.go b/modules/storage/local.go
@@ -103,7 +103,8 @@ func (l *LocalStorage) Save(path string, r io.Reader, size int64) (int64, error)
 		return 0, err
 	}
 	// Golang's tmp file (os.CreateTemp) always have 0o600 mode, so we need to change the file to follow the umask (as what Create/MkDir does)
-	if err := util.ApplyUmask(p, os.ModePerm); err != nil {
+	// but we don't want to make these files executable - so ensure that we mask out the executable bits
+	if err := util.ApplyUmask(p, os.ModePerm&0o666); err != nil {
 		return 0, err
 	}
 

Original file line number	Diff line number	Diff line change
`@@ -103,7 +103,8 @@ func (l *LocalStorage) Save(path string, r io.Reader, size int64) (int64, error)`
`103`	`103`	`return 0, err`
`104`	`104`	`}`
`105`	`105`	`// Golang's tmp file (os.CreateTemp) always have 0o600 mode, so we need to change the file to follow the umask (as what Create/MkDir does)`
`106`		`- if err := util.ApplyUmask(p, os.ModePerm); err != nil {`
	`106`	`+ // but we don't want to make these files executable - so ensure that we mask out the executable bits`
	`107`	`+ if err := util.ApplyUmask(p, os.ModePerm&0o666); err != nil {`
`107`	`108`	`return 0, err`
`108`	`109`	`}`
`109`	`110`