* Improved and more secure handling of paths and symbolic links.
authorUrban Wallasch <urban.wallasch@freenet.de>
Thu, 4 Apr 2019 14:13:24 +0000 (16:13 +0200)
committerUrban Wallasch <urban.wallasch@freenet.de>
Thu, 4 Apr 2019 14:13:24 +0000 (16:13 +0200)
* General cleanup and refactoring.

gogopherd.go

index 70caffe3d0d82135268c5d9b996f8530bb911856..44804c10cf287dde9aba1eb32f68cd1e9fa8aebc 100644 (file)
@@ -16,6 +16,7 @@ package main
 
 import (
        "bufio"
+       "errors"
        "flag"
        "fmt"
        "io"
@@ -23,14 +24,15 @@ import (
        "log"
        "net"
        "os"
+       "path/filepath"
        "strconv"
        "strings"
 )
 
 var (
-       index  []string
-       logger *log.Logger
-       tracer *log.Logger
+       logger  *log.Logger
+       tracer  *log.Logger
+       pathSep string
 )
 
 var cfg = struct {
@@ -68,6 +70,11 @@ func initialize() {
        } else {
                tracer = log.New(ioutil.Discard, "", 0)
        }
+       pathSep = string(os.PathSeparator)
+
+       var err error
+       cfg.docRoot, err = canonicalizePath(cfg.docRoot)
+       checkFatal(err, "canonicalizePath "+cfg.docRoot)
 
        tracer.Print("interface: ", cfg.iface)
        tracer.Print("TCP port:  ", cfg.port)
@@ -90,16 +97,30 @@ func check(err error, msg string) error {
        return err
 }
 
-func sanitizeSelector(selector string) string {
-       selector = strings.Replace(selector, "\r", "", -1)
-       selector = strings.Replace(selector, "\n", "", -1)
-       selector = strings.Replace(selector, "../", "/", -1)
-       selector = strings.Replace(selector, "//", "/", -1)
-       selector = strings.Replace(selector, "//", "/", -1)
-       return selector
+func canonicalizePath(path string) (string, error) {
+       evp, err := filepath.EvalSymlinks(path)
+       if check(err, "EvalSymlinks "+path) != nil {
+               return path, err
+       }
+       abs, err := filepath.Abs(evp)
+       if check(err, "Abs "+evp) != nil {
+               return path, err
+       }
+       return abs, err
+}
+
+func validatePath(relpath string) (string, error) {
+       path, err := canonicalizePath(cfg.docRoot + pathSep + relpath)
+       if check(err, "canonicalizePath "+relpath) != nil {
+               return path, err
+       }
+       if len(path) < len(cfg.docRoot) || path[:len(cfg.docRoot)] != cfg.docRoot {
+               return path, errors.New("Path outside docRoot")
+       }
+       return path, err
 }
 
-func makeIndex(selector string) (string, error) {
+func createIndex(selector string) (string, error) {
        dirname := cfg.docRoot + selector
        d, err := os.Open(dirname)
        defer d.Close()
@@ -110,14 +131,39 @@ func makeIndex(selector string) (string, error) {
        if check(err, "Readdir "+dirname) != nil {
                return "", err
        }
-       list := ""
        loc := "\t" + cfg.fqdn + "\t" + cfg.port + "\r\n"
+       list := "1HOME\t/" + loc
+       updir, _ := filepath.Split(selector)
+       list += "1..\t" + updir + loc
        for _, fi := range fi {
-               if fi.Mode().IsRegular() {
-                       list += "0" + fi.Name() + " (" + strconv.FormatInt(fi.Size(), 10) + ")\t" + selector + "/" + fi.Name() + loc
-               } else if fi.Mode().IsDir() {
+               fmode := fi.Mode()
+               if fmode.IsDir() {
+                       // create a directory reference
+                       list += "1" + fi.Name() + "\t" + selector + pathSep + fi.Name() + loc
+               } else if fmode.IsRegular() {
+                       // create a file reference
                        // TODO: determine file type
-                       list += "1" + fi.Name() + "\t" + selector + "/" + fi.Name() + loc
+                       list += "0" + fi.Name() + " (" + strconv.FormatInt(fi.Size(), 10) + ")\t" + selector + pathSep + fi.Name() + loc
+               } else if fmode&os.ModeSymlink != 0 {
+                       // create a reference according to link target
+                       linktarget, _ := os.Readlink(dirname + pathSep + fi.Name())
+                       path, err := validatePath(linktarget)
+                       if check(err, "validatePath "+path) == nil {
+                               tracer.Print("path: '", path, "'")
+                               lfi, err := os.Stat(path)
+                               if check(err, "Stat "+path) == nil {
+                                       if lfi.IsDir() {
+                                               // link points to a directory
+                                               list += "1" + fi.Name() + " -> " + linktarget + "\t" + selector + pathSep + fi.Name() + loc
+                                       } else if lfi.Mode().IsRegular() {
+                                               // link points to a regular file
+                                               // TODO: determine file type
+                                               list += "0" + fi.Name() + " -> " + linktarget + "\t" + selector + pathSep + fi.Name() + loc
+                                       }
+                               }
+                       }
+               } else {
+                       tracer.Print("unhandled file mode: " + fi.Name() + " (" + fmode.String() + ")")
                }
        }
        return list, error(nil)
@@ -125,44 +171,53 @@ func makeIndex(selector string) (string, error) {
 
 func handleRequest(conn net.Conn) {
        defer conn.Close()
+       // get request
        req, err := bufio.NewReader(conn).ReadString('\n')
        if check(err, "ReadString") != nil {
                return
        }
-       selector := sanitizeSelector("/" + req)
-       tracer.Print("selector: '", selector, "'")
-
-       path := cfg.docRoot + selector
+       req = strings.TrimSpace(req)
+       tracer.Print("request: '", req, "'")
+       // evaluate, canonicalize, and validate referenced path
+       path, err := validatePath(req)
+       if check(err, "validatePath "+path) != nil {
+               return
+       }
+       tracer.Print("path: '", path, "'")
        fi, err := os.Stat(path)
        if check(err, "Stat "+path) != nil {
                return
        }
-       var nb int64
-       if fi.IsDir() {
-               sres, err := makeIndex(selector)
+       fmode := fi.Mode()
+       // send appropriate resource (file or directory index)
+       selector := path[len(cfg.docRoot):]
+       tracer.Print("selector: '", selector, "'")
+       var nbytes int64
+       if fmode.IsDir() {
+               diridx, err := createIndex(selector)
                if check(err, "makeIndex "+path) != nil {
                        return
                }
-               b, err := conn.Write([]byte(cfg.header + sres))
+               nb, err := conn.Write([]byte(cfg.header + diridx))
                if check(err, "Write") != nil {
                        return
                }
-               nb = int64(b)
-       } else if fi.Mode().IsRegular() {
+               nbytes = int64(nb)
+       } else if fmode.IsRegular() {
                file, err := os.Open(path)
                if check(err, "Open "+path) != nil {
                        return
                }
                defer file.Close()
-               nb, err = io.Copy(conn, file)
+               nbytes, err = io.Copy(conn, file)
                if check(err, "Copy "+path) != nil {
                        return
                }
        } else {
                // TODO: handle other cases?
-               nb = -1
+               nbytes = -1
        }
-       tracer.Print(strconv.FormatInt(nb, 10) + " bytes sent.")
+       tracer.Print(strconv.FormatInt(nbytes, 10) + " bytes sent.")
        return
 }