time: bound file reads and validate LoadLocation argument
Fixes #18985
Change-Id: I956117f47d1d2b453b4786c7b78c1c944defeca0
Reviewed-on: https://go-review.googlesource.com/36551
Reviewed-by: Ian Lance Taylor <iant@golang.org>
diff --git a/src/time/export_test.go b/src/time/export_test.go
index 26584b5..4c08ab1 100644
--- a/src/time/export_test.go
+++ b/src/time/export_test.go
@@ -32,4 +32,6 @@
ParseTimeZone = parseTimeZone
SetMono = (*Time).setMono
GetMono = (*Time).mono
+ ErrLocation = errLocation
+ ReadFile = readFile
)
diff --git a/src/time/sys_plan9.go b/src/time/sys_plan9.go
index 11365a7..9086a6e 100644
--- a/src/time/sys_plan9.go
+++ b/src/time/sys_plan9.go
@@ -19,6 +19,7 @@
// readFile reads and returns the content of the named file.
// It is a trivial implementation of ioutil.ReadFile, reimplemented
// here to avoid depending on io/ioutil or os.
+// It returns an error if name exceeds maxFileSize bytes.
func readFile(name string) ([]byte, error) {
f, err := syscall.Open(name, syscall.O_RDONLY)
if err != nil {
@@ -38,6 +39,9 @@
if n == 0 || err != nil {
break
}
+ if len(ret) > maxFileSize {
+ return nil, fileSizeError(name)
+ }
}
return ret, err
}
diff --git a/src/time/sys_unix.go b/src/time/sys_unix.go
index 91d54c9..d4db8f9 100644
--- a/src/time/sys_unix.go
+++ b/src/time/sys_unix.go
@@ -19,6 +19,7 @@
// readFile reads and returns the content of the named file.
// It is a trivial implementation of ioutil.ReadFile, reimplemented
// here to avoid depending on io/ioutil or os.
+// It returns an error if name exceeds maxFileSize bytes.
func readFile(name string) ([]byte, error) {
f, err := syscall.Open(name, syscall.O_RDONLY, 0)
if err != nil {
@@ -38,6 +39,9 @@
if n == 0 || err != nil {
break
}
+ if len(ret) > maxFileSize {
+ return nil, fileSizeError(name)
+ }
}
return ret, err
}
diff --git a/src/time/sys_windows.go b/src/time/sys_windows.go
index a4a068f..9e38165 100644
--- a/src/time/sys_windows.go
+++ b/src/time/sys_windows.go
@@ -16,6 +16,7 @@
// readFile reads and returns the content of the named file.
// It is a trivial implementation of ioutil.ReadFile, reimplemented
// here to avoid depending on io/ioutil or os.
+// It returns an error if name exceeds maxFileSize bytes.
func readFile(name string) ([]byte, error) {
f, err := syscall.Open(name, syscall.O_RDONLY, 0)
if err != nil {
@@ -35,6 +36,9 @@
if n == 0 || err != nil {
break
}
+ if len(ret) > maxFileSize {
+ return nil, fileSizeError(name)
+ }
}
return ret, err
}
diff --git a/src/time/time_test.go b/src/time/time_test.go
index 2922560..90e2abf 100644
--- a/src/time/time_test.go
+++ b/src/time/time_test.go
@@ -11,7 +11,9 @@
"fmt"
"math/big"
"math/rand"
+ "os"
"runtime"
+ "strings"
"testing"
"testing/quick"
. "time"
@@ -1254,3 +1256,14 @@
t.Errorf("zero month = %q; want %q", got, want)
}
}
+
+func TestReadFileLimit(t *testing.T) {
+ const zero = "/dev/zero"
+ if _, err := os.Stat(zero); err != nil {
+ t.Skip("skipping test without a /dev/zero")
+ }
+ _, err := ReadFile(zero)
+ if err == nil || !strings.Contains(err.Error(), "is too large") {
+ t.Errorf("readFile(%q) error = %v; want error containing 'is too large'", zero, err)
+ }
+}
diff --git a/src/time/zoneinfo.go b/src/time/zoneinfo.go
index 7cde142..dfe857f 100644
--- a/src/time/zoneinfo.go
+++ b/src/time/zoneinfo.go
@@ -5,6 +5,7 @@
package time
import (
+ "errors"
"sync"
"syscall"
)
@@ -256,6 +257,8 @@
// NOTE(rsc): Eventually we will need to accept the POSIX TZ environment
// syntax too, but I don't feel like implementing it today.
+var errLocation = errors.New("time: invalid location name")
+
var zoneinfo *string
var zoneinfoOnce sync.Once
@@ -280,6 +283,11 @@
if name == "Local" {
return Local, nil
}
+ if containsDotDot(name) || name[0] == '/' || name[0] == '\\' {
+ // No valid IANA Time Zone name contains a single dot,
+ // much less dot dot. Likewise, none begin with a slash.
+ return nil, errLocation
+ }
zoneinfoOnce.Do(func() {
env, _ := syscall.Getenv("ZONEINFO")
zoneinfo = &env
@@ -292,3 +300,16 @@
}
return loadLocation(name)
}
+
+// containsDotDot reports whether s contains "..".
+func containsDotDot(s string) bool {
+ if len(s) < 2 {
+ return false
+ }
+ for i := 0; i < len(s)-1; i++ {
+ if s[i] == '.' && s[i+1] == '.' {
+ return true
+ }
+ }
+ return false
+}
diff --git a/src/time/zoneinfo_read.go b/src/time/zoneinfo_read.go
index 1b3356e..b0cd9da 100644
--- a/src/time/zoneinfo_read.go
+++ b/src/time/zoneinfo_read.go
@@ -11,6 +11,17 @@
import "errors"
+// maxFileSize is the max permitted size of files read by readFile.
+// As reference, the zoneinfo.zip distributed by Go is ~350 KB,
+// so 10MB is overkill.
+const maxFileSize = 10 << 20
+
+type fileSizeError string
+
+func (f fileSizeError) Error() string {
+ return "time: file " + string(f) + " is too large"
+}
+
// Copies of io.Seek* constants to avoid importing "io":
const (
seekStart = 0
diff --git a/src/time/zoneinfo_test.go b/src/time/zoneinfo_test.go
index e388e99..b25733c 100644
--- a/src/time/zoneinfo_test.go
+++ b/src/time/zoneinfo_test.go
@@ -20,8 +20,8 @@
func TestEnvVarUsage(t *testing.T) {
time.ResetZoneinfoForTesting()
- testZoneinfo := "foo.zip"
- env := "ZONEINFO"
+ const testZoneinfo = "foo.zip"
+ const env = "ZONEINFO"
defer os.Setenv(env, os.Getenv(env))
os.Setenv(env, testZoneinfo)
@@ -35,6 +35,26 @@
}
}
+func TestLoadLocationValidatesNames(t *testing.T) {
+ time.ResetZoneinfoForTesting()
+ const env = "ZONEINFO"
+ defer os.Setenv(env, os.Getenv(env))
+ os.Setenv(env, "")
+
+ bad := []string{
+ "/usr/foo/Foo",
+ "\\UNC\foo",
+ "..",
+ "a..",
+ }
+ for _, v := range bad {
+ _, err := time.LoadLocation(v)
+ if err != time.ErrLocation {
+ t.Errorf("LoadLocation(%q) error = %v; want ErrLocation", v, err)
+ }
+ }
+}
+
func TestVersion3(t *testing.T) {
time.ForceZipFileForTesting(true)
defer time.ForceZipFileForTesting(false)
