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 26584b5454be28fef1b141293b44510c12e6d45b..4c08ab13afc27276d76d9b08c268683eb686d1fa 100644 (file)
--- a/src/time/export_test.go
+++ b/src/time/export_test.go
@@ -32,4 +32,6 @@ var (
        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 11365a791f7dfc48fd20b6affff042ae566fda37..9086a6e835ff9aa89daec0e760e282e1a31c572f 100644 (file)
--- a/src/time/sys_plan9.go
+++ b/src/time/sys_plan9.go
@@ -19,6 +19,7 @@ func interrupt() {
 // 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 @@ func readFile(name string) ([]byte, error) {
                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 91d54c9ffd0a9e947fa5b7367abeb2c10359d0c1..d4db8f9afd5f846d7d2895c687ca4159bbac4b7e 100644 (file)
--- a/src/time/sys_unix.go
+++ b/src/time/sys_unix.go
@@ -19,6 +19,7 @@ func interrupt() {
 // 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 @@ func readFile(name string) ([]byte, error) {
                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 a4a068f78494c101f455dee014ca3faa2e44fb1b..9e381653937cc6225bf4b821dfab6ce6ae7b6d19 100644 (file)
--- a/src/time/sys_windows.go
+++ b/src/time/sys_windows.go
@@ -16,6 +16,7 @@ func interrupt() {
 // 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 @@ func readFile(name string) ([]byte, error) {
                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 2922560f0972b0b9c991a96f6eabc467723f4b53..90e2abf03efdb6b49ce885def816f379e5536a64 100644 (file)
--- a/src/time/time_test.go
+++ b/src/time/time_test.go
@@ -11,7 +11,9 @@ import (
        "fmt"
        "math/big"
        "math/rand"
+       "os"
        "runtime"
+       "strings"
        "testing"
        "testing/quick"
        . "time"
@@ -1254,3 +1256,14 @@ func TestZeroMonthString(t *testing.T) {
                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 7cde142239cb1cdd5f8c00e16a7ae51fa77491ce..dfe857f436df98fc63eb7107f78c7937422856e8 100644 (file)
--- a/src/time/zoneinfo.go
+++ b/src/time/zoneinfo.go
@@ -5,6 +5,7 @@
 package time
 
 import (
+       "errors"
        "sync"
        "syscall"
 )
@@ -256,6 +257,8 @@ func (l *Location) lookupName(name string, unix int64) (offset int, isDST bool,
 // 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 @@ func LoadLocation(name string) (*Location, error) {
        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 @@ func LoadLocation(name string) (*Location, error) {
        }
        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 1b3356e48ca33d0ef49001cde2af22f78cd53932..b0cd9da923062a38ccf7d28f37ac2b1575e8e9c8 100644 (file)
--- a/src/time/zoneinfo_read.go
+++ b/src/time/zoneinfo_read.go
@@ -11,6 +11,17 @@ package time
 
 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 e388e998d07d02cf09fe8c35dd9d67cf61901c75..b25733c1f6974981e67cd12b3f416fd1d73fb5be 100644 (file)
--- a/src/time/zoneinfo_test.go
+++ b/src/time/zoneinfo_test.go
@@ -20,8 +20,8 @@ func init() {
 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 TestEnvVarUsage(t *testing.T) {
        }
 }
 
+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)