Commit c6709d93 authored by Yong Tang's avatar Yong Tang Committed by GitHub

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: default avatarYong Tang <yong.tang.github@outlook.com>
parent b40f2a0a
...@@ -6,6 +6,7 @@ import ( ...@@ -6,6 +6,7 @@ import (
"fmt" "fmt"
"log" "log"
"os" "os"
"path/filepath"
"runtime" "runtime"
"strings" "strings"
...@@ -95,7 +96,7 @@ func confLoader(serverType string) (caddy.Input, error) { ...@@ -95,7 +96,7 @@ func confLoader(serverType string) (caddy.Input, error) {
return caddy.CaddyfileFromPipe(os.Stdin, serverType) return caddy.CaddyfileFromPipe(os.Stdin, serverType)
} }
contents, err := os.ReadFile(conf) contents, err := os.ReadFile(filepath.Clean(conf))
if err != nil { if err != nil {
return nil, err return nil, err
} }
......
...@@ -37,7 +37,7 @@ func (a Auto) Walk() error { ...@@ -37,7 +37,7 @@ func (a Auto) Walk() error {
return nil return nil
} }
reader, err := os.Open(path) reader, err := os.Open(filepath.Clean(path))
if err != nil { if err != nil {
log.Warningf("Opening %s failed: %s", path, err) log.Warningf("Opening %s failed: %s", path, err)
return nil return nil
......
...@@ -6,6 +6,7 @@ import ( ...@@ -6,6 +6,7 @@ import (
"crypto/rsa" "crypto/rsa"
"errors" "errors"
"os" "os"
"path/filepath"
"time" "time"
"github.com/coredns/coredns/request" "github.com/coredns/coredns/request"
...@@ -25,7 +26,7 @@ type DNSKEY struct { ...@@ -25,7 +26,7 @@ type DNSKEY struct {
// ParseKeyFile read a DNSSEC keyfile as generated by dnssec-keygen or other // 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. // utilities. It adds ".key" for the public key and ".private" for the private key.
func ParseKeyFile(pubFile, privFile string) (*DNSKEY, error) { func ParseKeyFile(pubFile, privFile string) (*DNSKEY, error) {
f, e := os.Open(pubFile) f, e := os.Open(filepath.Clean(pubFile))
if e != nil { if e != nil {
return nil, e return nil, e
} }
...@@ -35,7 +36,7 @@ func ParseKeyFile(pubFile, privFile string) (*DNSKEY, error) { ...@@ -35,7 +36,7 @@ func ParseKeyFile(pubFile, privFile string) (*DNSKEY, error) {
return nil, e return nil, e
} }
f, e = os.Open(privFile) f, e = os.Open(filepath.Clean(privFile))
if e != nil { if e != nil {
return nil, e return nil, e
} }
......
...@@ -2,6 +2,7 @@ package file ...@@ -2,6 +2,7 @@ package file
import ( import (
"os" "os"
"path/filepath"
"time" "time"
"github.com/coredns/coredns/plugin/transfer" "github.com/coredns/coredns/plugin/transfer"
...@@ -19,7 +20,7 @@ func (z *Zone) Reload(t *transfer.Transfer) error { ...@@ -19,7 +20,7 @@ func (z *Zone) Reload(t *transfer.Transfer) error {
select { select {
case <-tick.C: case <-tick.C:
zFile := z.File() zFile := z.File()
reader, err := os.Open(zFile) reader, err := os.Open(filepath.Clean(zFile))
if err != nil { if err != nil {
log.Errorf("Failed to open zone %q in %q: %v", z.origin, zFile, err) log.Errorf("Failed to open zone %q in %q: %v", z.origin, zFile, err)
continue continue
......
...@@ -88,7 +88,7 @@ func fileParse(c *caddy.Controller) (Zones, error) { ...@@ -88,7 +88,7 @@ func fileParse(c *caddy.Controller) (Zones, error) {
fileName = filepath.Join(config.Root, fileName) fileName = filepath.Join(config.Root, fileName)
} }
reader, err := os.Open(fileName) reader, err := os.Open(filepath.Clean(fileName))
if err != nil { if err != nil {
openErr = err openErr = err
} }
......
...@@ -7,6 +7,7 @@ import ( ...@@ -7,6 +7,7 @@ import (
"net" "net"
"net/http" "net/http"
"os" "os"
"path/filepath"
"time" "time"
) )
...@@ -95,7 +96,7 @@ func loadRoots(caPath string) (*x509.CertPool, error) { ...@@ -95,7 +96,7 @@ func loadRoots(caPath string) (*x509.CertPool, error) {
} }
roots := x509.NewCertPool() roots := x509.NewCertPool()
pem, err := os.ReadFile(caPath) pem, err := os.ReadFile(filepath.Clean(caPath))
if err != nil { if err != nil {
return nil, fmt.Errorf("error reading %s: %s", caPath, err) return nil, fmt.Errorf("error reading %s: %s", caPath, err)
} }
......
...@@ -66,7 +66,7 @@ func keyParse(c *caddy.Controller) ([]Pair, error) { ...@@ -66,7 +66,7 @@ func keyParse(c *caddy.Controller) ([]Pair, error) {
} }
func readKeyPair(public, private string) (Pair, error) { func readKeyPair(public, private string) (Pair, error) {
rk, err := os.Open(public) rk, err := os.Open(filepath.Clean(public))
if err != nil { if err != nil {
return Pair{}, err return Pair{}, err
} }
...@@ -86,7 +86,7 @@ func readKeyPair(public, private string) (Pair, error) { ...@@ -86,7 +86,7 @@ func readKeyPair(public, private string) (Pair, error) {
return Pair{}, fmt.Errorf("DNSKEY in %q is not a CSK/KSK", public) 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 { if err != nil {
return Pair{}, err return Pair{}, err
} }
......
...@@ -111,7 +111,7 @@ func (s *Signer) Sign(now time.Time) (*file.Zone, error) { ...@@ -111,7 +111,7 @@ func (s *Signer) Sign(now time.Time) (*file.Zone, error) {
// resign checks if the signed zone exists, or needs resigning. // resign checks if the signed zone exists, or needs resigning.
func (s *Signer) resign() error { func (s *Signer) resign() error {
signedfile := filepath.Join(s.directory, s.signedfile) 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) { if err != nil && os.IsNotExist(err) {
return err return err
} }
......
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