Commit bd561699 authored by Brad Fitzpatrick's avatar Brad Fitzpatrick

time: bound file reads and validate LoadLocation argument

Fixes #18985

Change-Id: I956117f47d1d2b453b4786c7b78c1c944defeca0
Reviewed-on: https://go-review.googlesource.com/36551Reviewed-by: default avatarIan Lance Taylor <iant@golang.org>
parent e410d2a8
...@@ -32,4 +32,6 @@ var ( ...@@ -32,4 +32,6 @@ var (
ParseTimeZone = parseTimeZone ParseTimeZone = parseTimeZone
SetMono = (*Time).setMono SetMono = (*Time).setMono
GetMono = (*Time).mono GetMono = (*Time).mono
ErrLocation = errLocation
ReadFile = readFile
) )
...@@ -19,6 +19,7 @@ func interrupt() { ...@@ -19,6 +19,7 @@ func interrupt() {
// readFile reads and returns the content of the named file. // readFile reads and returns the content of the named file.
// It is a trivial implementation of ioutil.ReadFile, reimplemented // It is a trivial implementation of ioutil.ReadFile, reimplemented
// here to avoid depending on io/ioutil or os. // here to avoid depending on io/ioutil or os.
// It returns an error if name exceeds maxFileSize bytes.
func readFile(name string) ([]byte, error) { func readFile(name string) ([]byte, error) {
f, err := syscall.Open(name, syscall.O_RDONLY) f, err := syscall.Open(name, syscall.O_RDONLY)
if err != nil { if err != nil {
...@@ -38,6 +39,9 @@ func readFile(name string) ([]byte, error) { ...@@ -38,6 +39,9 @@ func readFile(name string) ([]byte, error) {
if n == 0 || err != nil { if n == 0 || err != nil {
break break
} }
if len(ret) > maxFileSize {
return nil, fileSizeError(name)
}
} }
return ret, err return ret, err
} }
......
...@@ -19,6 +19,7 @@ func interrupt() { ...@@ -19,6 +19,7 @@ func interrupt() {
// readFile reads and returns the content of the named file. // readFile reads and returns the content of the named file.
// It is a trivial implementation of ioutil.ReadFile, reimplemented // It is a trivial implementation of ioutil.ReadFile, reimplemented
// here to avoid depending on io/ioutil or os. // here to avoid depending on io/ioutil or os.
// It returns an error if name exceeds maxFileSize bytes.
func readFile(name string) ([]byte, error) { func readFile(name string) ([]byte, error) {
f, err := syscall.Open(name, syscall.O_RDONLY, 0) f, err := syscall.Open(name, syscall.O_RDONLY, 0)
if err != nil { if err != nil {
...@@ -38,6 +39,9 @@ func readFile(name string) ([]byte, error) { ...@@ -38,6 +39,9 @@ func readFile(name string) ([]byte, error) {
if n == 0 || err != nil { if n == 0 || err != nil {
break break
} }
if len(ret) > maxFileSize {
return nil, fileSizeError(name)
}
} }
return ret, err return ret, err
} }
......
...@@ -16,6 +16,7 @@ func interrupt() { ...@@ -16,6 +16,7 @@ func interrupt() {
// readFile reads and returns the content of the named file. // readFile reads and returns the content of the named file.
// It is a trivial implementation of ioutil.ReadFile, reimplemented // It is a trivial implementation of ioutil.ReadFile, reimplemented
// here to avoid depending on io/ioutil or os. // here to avoid depending on io/ioutil or os.
// It returns an error if name exceeds maxFileSize bytes.
func readFile(name string) ([]byte, error) { func readFile(name string) ([]byte, error) {
f, err := syscall.Open(name, syscall.O_RDONLY, 0) f, err := syscall.Open(name, syscall.O_RDONLY, 0)
if err != nil { if err != nil {
...@@ -35,6 +36,9 @@ func readFile(name string) ([]byte, error) { ...@@ -35,6 +36,9 @@ func readFile(name string) ([]byte, error) {
if n == 0 || err != nil { if n == 0 || err != nil {
break break
} }
if len(ret) > maxFileSize {
return nil, fileSizeError(name)
}
} }
return ret, err return ret, err
} }
......
...@@ -11,7 +11,9 @@ import ( ...@@ -11,7 +11,9 @@ import (
"fmt" "fmt"
"math/big" "math/big"
"math/rand" "math/rand"
"os"
"runtime" "runtime"
"strings"
"testing" "testing"
"testing/quick" "testing/quick"
. "time" . "time"
...@@ -1254,3 +1256,14 @@ func TestZeroMonthString(t *testing.T) { ...@@ -1254,3 +1256,14 @@ func TestZeroMonthString(t *testing.T) {
t.Errorf("zero month = %q; want %q", got, want) 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)
}
}
...@@ -5,6 +5,7 @@ ...@@ -5,6 +5,7 @@
package time package time
import ( import (
"errors"
"sync" "sync"
"syscall" "syscall"
) )
...@@ -256,6 +257,8 @@ func (l *Location) lookupName(name string, unix int64) (offset int, isDST bool, ...@@ -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 // NOTE(rsc): Eventually we will need to accept the POSIX TZ environment
// syntax too, but I don't feel like implementing it today. // syntax too, but I don't feel like implementing it today.
var errLocation = errors.New("time: invalid location name")
var zoneinfo *string var zoneinfo *string
var zoneinfoOnce sync.Once var zoneinfoOnce sync.Once
...@@ -280,6 +283,11 @@ func LoadLocation(name string) (*Location, error) { ...@@ -280,6 +283,11 @@ func LoadLocation(name string) (*Location, error) {
if name == "Local" { if name == "Local" {
return Local, nil 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() { zoneinfoOnce.Do(func() {
env, _ := syscall.Getenv("ZONEINFO") env, _ := syscall.Getenv("ZONEINFO")
zoneinfo = &env zoneinfo = &env
...@@ -292,3 +300,16 @@ func LoadLocation(name string) (*Location, error) { ...@@ -292,3 +300,16 @@ func LoadLocation(name string) (*Location, error) {
} }
return loadLocation(name) 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
}
...@@ -11,6 +11,17 @@ package time ...@@ -11,6 +11,17 @@ package time
import "errors" 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": // Copies of io.Seek* constants to avoid importing "io":
const ( const (
seekStart = 0 seekStart = 0
......
...@@ -20,8 +20,8 @@ func init() { ...@@ -20,8 +20,8 @@ func init() {
func TestEnvVarUsage(t *testing.T) { func TestEnvVarUsage(t *testing.T) {
time.ResetZoneinfoForTesting() time.ResetZoneinfoForTesting()
testZoneinfo := "foo.zip" const testZoneinfo = "foo.zip"
env := "ZONEINFO" const env = "ZONEINFO"
defer os.Setenv(env, os.Getenv(env)) defer os.Setenv(env, os.Getenv(env))
os.Setenv(env, testZoneinfo) os.Setenv(env, testZoneinfo)
...@@ -35,6 +35,26 @@ func TestEnvVarUsage(t *testing.T) { ...@@ -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) { func TestVersion3(t *testing.T) {
time.ForceZipFileForTesting(true) time.ForceZipFileForTesting(true)
defer time.ForceZipFileForTesting(false) defer time.ForceZipFileForTesting(false)
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment