From 52c70c763c73e2bf425d0c26483aa4c6b2707dd8 Mon Sep 17 00:00:00 2001 From: Urban Wallasch Date: Thu, 4 Apr 2019 16:13:24 +0200 Subject: [PATCH] * Improved and more secure handling of paths and symbolic links. * General cleanup and refactoring. --- gogopherd.go | 113 ++++++++++++++++++++++++++++++++++++++------------- 1 file changed, 84 insertions(+), 29 deletions(-) diff --git a/gogopherd.go b/gogopherd.go index 70caffe..44804c1 100644 --- a/gogopherd.go +++ b/gogopherd.go @@ -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 } -- 2.30.2