From c6709d930f71d64dc3b5d1a15943e5c927e808cc Mon Sep 17 00:00:00 2001 From: Yong Tang Date: Mon, 14 Feb 2022 08:24:21 -0800 Subject: [PATCH] Fix security scans by cleaning up file path (#5185) While performing security scans there were several issue raised as G304 (CWE-22): Potential file inclusion via variable. As some files path are taken from user input, it is possible the filepath passed by user may have unintended effect if not properly formed. This fix add Clean to remove the security warning and address some potential issue. Signed-off-by: Yong Tang --- coremain/run.go | 3 ++- plugin/auto/walk.go | 2 +- plugin/dnssec/dnskey.go | 5 +++-- plugin/file/reload.go | 3 ++- plugin/file/setup.go | 2 +- plugin/pkg/tls/tls.go | 3 ++- plugin/sign/keys.go | 4 ++-- plugin/sign/signer.go | 2 +- 8 files changed, 14 insertions(+), 10 deletions(-) diff --git a/coremain/run.go b/coremain/run.go index ff5c5ed56..fa7657886 100644 --- a/coremain/run.go +++ b/coremain/run.go @@ -6,6 +6,7 @@ import ( "fmt" "log" "os" + "path/filepath" "runtime" "strings" @@ -95,7 +96,7 @@ func confLoader(serverType string) (caddy.Input, error) { return caddy.CaddyfileFromPipe(os.Stdin, serverType) } - contents, err := os.ReadFile(conf) + contents, err := os.ReadFile(filepath.Clean(conf)) if err != nil { return nil, err } diff --git a/plugin/auto/walk.go b/plugin/auto/walk.go index 9108fe941..993ded408 100644 --- a/plugin/auto/walk.go +++ b/plugin/auto/walk.go @@ -37,7 +37,7 @@ func (a Auto) Walk() error { return nil } - reader, err := os.Open(path) + reader, err := os.Open(filepath.Clean(path)) if err != nil { log.Warningf("Opening %s failed: %s", path, err) return nil diff --git a/plugin/dnssec/dnskey.go b/plugin/dnssec/dnskey.go index 11e18fdc6..161db9471 100644 --- a/plugin/dnssec/dnskey.go +++ b/plugin/dnssec/dnskey.go @@ -6,6 +6,7 @@ import ( "crypto/rsa" "errors" "os" + "path/filepath" "time" "github.com/coredns/coredns/request" @@ -25,7 +26,7 @@ type DNSKEY struct { // ParseKeyFile read a DNSSEC keyfile as generated by dnssec-keygen or other // utilities. It adds ".key" for the public key and ".private" for the private key. func ParseKeyFile(pubFile, privFile string) (*DNSKEY, error) { - f, e := os.Open(pubFile) + f, e := os.Open(filepath.Clean(pubFile)) if e != nil { return nil, e } @@ -35,7 +36,7 @@ func ParseKeyFile(pubFile, privFile string) (*DNSKEY, error) { return nil, e } - f, e = os.Open(privFile) + f, e = os.Open(filepath.Clean(privFile)) if e != nil { return nil, e } diff --git a/plugin/file/reload.go b/plugin/file/reload.go index a154c0467..cdb50f439 100644 --- a/plugin/file/reload.go +++ b/plugin/file/reload.go @@ -2,6 +2,7 @@ package file import ( "os" + "path/filepath" "time" "github.com/coredns/coredns/plugin/transfer" @@ -19,7 +20,7 @@ func (z *Zone) Reload(t *transfer.Transfer) error { select { case <-tick.C: zFile := z.File() - reader, err := os.Open(zFile) + reader, err := os.Open(filepath.Clean(zFile)) if err != nil { log.Errorf("Failed to open zone %q in %q: %v", z.origin, zFile, err) continue diff --git a/plugin/file/setup.go b/plugin/file/setup.go index c9b8ddf3e..b8985f2d2 100644 --- a/plugin/file/setup.go +++ b/plugin/file/setup.go @@ -88,7 +88,7 @@ func fileParse(c *caddy.Controller) (Zones, error) { fileName = filepath.Join(config.Root, fileName) } - reader, err := os.Open(fileName) + reader, err := os.Open(filepath.Clean(fileName)) if err != nil { openErr = err } diff --git a/plugin/pkg/tls/tls.go b/plugin/pkg/tls/tls.go index dae509204..8d36d6823 100644 --- a/plugin/pkg/tls/tls.go +++ b/plugin/pkg/tls/tls.go @@ -7,6 +7,7 @@ import ( "net" "net/http" "os" + "path/filepath" "time" ) @@ -95,7 +96,7 @@ func loadRoots(caPath string) (*x509.CertPool, error) { } roots := x509.NewCertPool() - pem, err := os.ReadFile(caPath) + pem, err := os.ReadFile(filepath.Clean(caPath)) if err != nil { return nil, fmt.Errorf("error reading %s: %s", caPath, err) } diff --git a/plugin/sign/keys.go b/plugin/sign/keys.go index 5fd1a4842..b99958442 100644 --- a/plugin/sign/keys.go +++ b/plugin/sign/keys.go @@ -66,7 +66,7 @@ func keyParse(c *caddy.Controller) ([]Pair, error) { } func readKeyPair(public, private string) (Pair, error) { - rk, err := os.Open(public) + rk, err := os.Open(filepath.Clean(public)) if err != nil { return Pair{}, err } @@ -86,7 +86,7 @@ func readKeyPair(public, private string) (Pair, error) { return Pair{}, fmt.Errorf("DNSKEY in %q is not a CSK/KSK", public) } - rp, err := os.Open(private) + rp, err := os.Open(filepath.Clean(private)) if err != nil { return Pair{}, err } diff --git a/plugin/sign/signer.go b/plugin/sign/signer.go index d1ea22356..95ce94b52 100644 --- a/plugin/sign/signer.go +++ b/plugin/sign/signer.go @@ -111,7 +111,7 @@ func (s *Signer) Sign(now time.Time) (*file.Zone, error) { // resign checks if the signed zone exists, or needs resigning. func (s *Signer) resign() error { signedfile := filepath.Join(s.directory, s.signedfile) - rd, err := os.Open(signedfile) + rd, err := os.Open(filepath.Clean(signedfile)) if err != nil && os.IsNotExist(err) { return err }