From 887b49be325e82bdcaf4ea2023e003792323087f Mon Sep 17 00:00:00 2001 From: Martin Hebnes Pedersen Date: Fri, 22 Dec 2023 01:01:51 +0100 Subject: [PATCH] Hardning related to resolving Forms files Improved error handling, more efficient search and make sure we don't escape the Forms root directory. Also adds debug logging and fixes some reply templates we missed before. --- http.go | 18 ++-- internal/directories/directories.go | 15 +++ internal/forms/forms.go | 161 +++++++++++++++++++--------- 3 files changed, 134 insertions(+), 60 deletions(-) diff --git a/http.go b/http.go index 11dea589..2bcd0800 100644 --- a/http.go +++ b/http.go @@ -32,6 +32,7 @@ import ( "github.com/la5nta/wl2k-go/transport/ardop" "github.com/la5nta/pat/internal/buildinfo" + "github.com/la5nta/pat/internal/directories" "github.com/la5nta/pat/internal/gpsd" "github.com/gorilla/mux" @@ -216,11 +217,6 @@ func postPositionHandler(w http.ResponseWriter, r *http.Request) { } } -func isInPath(base string, path string) error { - _, err := filepath.Rel(base, path) - return err -} - func postMessageHandler(w http.ResponseWriter, r *http.Request) { box := mux.Vars(r)["box"] if box == "out" { @@ -239,9 +235,9 @@ func postMessageHandler(w http.ResponseWriter, r *http.Request) { // Check that we don't escape our mailbox path srcPath = filepath.Clean(srcPath) - if err := isInPath(mbox.MBoxPath, srcPath); err != nil { - log.Println("Malicious source path in move:", err) - http.Error(w, err.Error(), http.StatusBadRequest) + if !directories.IsInPath(mbox.MBoxPath, srcPath) { + log.Println("Malicious source path in move:", srcPath) + http.Error(w, "malicious source path", http.StatusBadRequest) return } @@ -678,9 +674,9 @@ func messageDeleteHandler(w http.ResponseWriter, r *http.Request) { box, mid := mux.Vars(r)["box"], mux.Vars(r)["mid"] file := filepath.Clean(filepath.Join(mbox.MBoxPath, box, mid+mailbox.Ext)) - if err := isInPath(mbox.MBoxPath, file); err != nil { - log.Println("Malicious source path in move:", err) - http.Error(w, err.Error(), http.StatusBadRequest) + if !directories.IsInPath(mbox.MBoxPath, file) { + log.Println("Malicious source path in move:", file) + http.Error(w, "malicious source path", http.StatusBadRequest) return } diff --git a/internal/directories/directories.go b/internal/directories/directories.go index f40fb1f2..c451eccb 100644 --- a/internal/directories/directories.go +++ b/internal/directories/directories.go @@ -21,6 +21,21 @@ var ( statePath string ) +// IsInPath returns true if sub is a sub-path of parent. +// +// Both paths must be either absolute or relative. +func IsInPath(parent, sub string) bool { + parent, sub = filepath.Clean(parent), filepath.Clean(sub) + if filepath.IsAbs(parent) != filepath.IsAbs(sub) { + panic("mix of rel and abs paths") + } + rel, err := filepath.Rel(parent, sub) + if err != nil { + return false + } + return rel != ".." && !strings.HasPrefix(rel, ".."+string(filepath.Separator)) +} + func DataDir() string { return getDir(&dataPath, xdg.DataHome, "DataDir") } diff --git a/internal/forms/forms.go b/internal/forms/forms.go index 3eaadfd1..86513207 100644 --- a/internal/forms/forms.go +++ b/internal/forms/forms.go @@ -34,6 +34,7 @@ import ( "github.com/dimchansky/utfbom" "github.com/la5nta/pat/cfg" "github.com/la5nta/pat/internal/debug" + "github.com/la5nta/pat/internal/directories" "github.com/la5nta/pat/internal/gpsd" "github.com/pd0mz/go-maidenhead" ) @@ -241,16 +242,9 @@ func (m *Manager) GetFormTemplateHandler(w http.ResponseWriter, r *http.Request) return } - absPathTemplate, err := m.findAbsPathForTemplatePath(formPath) + responseText, err := m.fillFormTemplate(formPath, "/api/form?"+r.URL.Query().Encode(), nil, make(map[string]string)) if err != nil { - http.Error(w, "find the full path for requested template "+formPath, http.StatusBadRequest) - log.Printf("find the full path for requested template %s %s: %s", r.Method, r.URL.Path, "can't open template "+formPath) - return - } - - responseText, err := m.fillFormTemplate(absPathTemplate, "/api/form?"+r.URL.Query().Encode(), nil, make(map[string]string)) - if err != nil { - http.Error(w, "can't open template "+formPath, http.StatusBadRequest) + http.Error(w, err.Error(), http.StatusBadRequest) log.Printf("problem filling form template file %s %s: can't open template %s. Err: %s", r.Method, r.URL.Path, formPath, err) return } @@ -485,12 +479,7 @@ func (m *Manager) RenderForm(contentUnsanitized []byte, composeReply bool) (stri formRelPath = form.ViewerURI } - absPathTemplate, err := m.findAbsPathForTemplatePath(formRelPath) - if err != nil { - return "", err - } - - return m.fillFormTemplate(absPathTemplate, "/api/form?composereply=true&formPath="+formRelPath, regexp.MustCompile(`{[vV][aA][rR]\s+(\w+)\s*}`), formVars) + return m.fillFormTemplate(formRelPath, "/api/form?composereply=true&formPath="+formRelPath, regexp.MustCompile(`{[vV][aA][rR]\s+(\w+)\s*}`), formVars) } // ComposeForm combines all data needed for the whole form-based message: subject, body, and attachment @@ -582,7 +571,7 @@ func (m *Manager) innerRecursiveBuildFormFolder(rootPath string) (FormFolder, er formCnt := 0 for _, info := range infos { if info.IsDir() { - subfolder, err := m.innerRecursiveBuildFormFolder(path.Join(rootPath, info.Name())) + subfolder, err := m.innerRecursiveBuildFormFolder(filepath.Join(rootPath, info.Name())) if err != nil { return folder, err } @@ -593,8 +582,10 @@ func (m *Manager) innerRecursiveBuildFormFolder(rootPath string) (FormFolder, er if !strings.EqualFold(filepath.Ext(info.Name()), txtFileExt) { continue } - frm, err := m.buildFormFromTxt(path.Join(rootPath, info.Name())) + path := m.rel(filepath.Join(rootPath, info.Name())) + frm, err := m.buildFormFromTxt(path) if err != nil { + debug.Printf("failed to load form file %q: %v", path, err) continue } if frm.InitialURI != "" || frm.ViewerURI != "" { @@ -612,18 +603,72 @@ func (m *Manager) innerRecursiveBuildFormFolder(rootPath string) (FormFolder, er return folder, nil } +// abs returns the absolute path of a path relative to m.FormsPath. +func (m *Manager) abs(path string) string { + if filepath.IsAbs(path) { + return path + } + return filepath.Join(m.config.FormsPath, path) +} + +// resolveFileReference searches for files referenced in .txt files. +// +// If found the returned path is relative to FormsPath and bool is true, otherwise the given path is returned unmodified. +func (m *Manager) resolveFileReference(path string) (string, bool) { + path = m.abs(path) + if _, err := os.Stat(path); err == nil { + return m.rel(path), true + } + // Fallback to case-insenstive search. + // Some HTML files references in the .txt files has a different caseness than the actual filename on disk. + absPathTemplateFolder := filepath.Dir(path) + entries, err := os.ReadDir(absPathTemplateFolder) + if err != nil { + return m.rel(path), false + } + for _, entry := range entries { + name := entry.Name() + if strings.EqualFold(filepath.Base(path), name) { + return m.rel(filepath.Join(absPathTemplateFolder, name)), true + } + } + return m.rel(path), false +} + +// rel returns a path relative to m.FormsPath. +func (m *Manager) rel(path string) string { + if !filepath.IsAbs(path) { + return path + } + rel, err := filepath.Rel(m.config.FormsPath, path) + if err != nil { + panic(err) + } + return rel +} + +// open opens a file with absolute path or path relative to m.FormsPath. +// +// An error is returned if the given path escapes m.FromsPath. +func (m *Manager) open(path string) (*os.File, error) { + absPath := m.abs(path) + // Make sure we doesn't escape FormsPath + if !directories.IsInPath(m.config.FormsPath, absPath) { + return nil, fmt.Errorf("%s escapes forms directory", path) + } + return os.Open(absPath) +} + func (m *Manager) buildFormFromTxt(txtPath string) (Form, error) { - f, err := os.Open(txtPath) + f, err := m.open(txtPath) if err != nil { return Form{}, err } defer f.Close() - formsPathWithSlash := m.config.FormsPath + "/" - form := Form{ Name: strings.TrimSuffix(path.Base(txtPath), path.Ext(txtPath)), - TxtFileURI: strings.TrimPrefix(txtPath, formsPathWithSlash), + TxtFileURI: txtPath, } scanner := bufio.NewScanner(f) baseURI := path.Dir(form.TxtFileURI) @@ -634,19 +679,41 @@ func (m *Manager) buildFormFromTxt(txtPath string) (Form, error) { // Form: , files := strings.Split(strings.TrimPrefix(l, "Form:"), ",") // Extend to absolute paths and add missing html extension - for i := range files { - files[i] = path.Join(baseURI, strings.TrimSpace(files[i])) - if ext := path.Ext(files[i]); ext == "" { + for i, name := range files { + files[i] = filepath.Join(baseURI, strings.TrimSpace(name)) + if ext := filepath.Ext(files[i]); ext == "" { files[i] += ".html" } + var ok bool + files[i], ok = m.resolveFileReference(files[i]) + if !ok { + debug.Printf("%s: failed to resolve referenced file %q", form.TxtFileURI, name) + } } form.InitialURI = files[0] if len(files) > 1 { form.ViewerURI = files[1] } case strings.HasPrefix(l, "ReplyTemplate:"): - form.ReplyTxtFileURI = path.Join(baseURI, strings.TrimSpace(strings.TrimPrefix(l, "ReplyTemplate:"))) - tmpForm, _ := m.buildFormFromTxt(path.Join(m.config.FormsPath, form.ReplyTxtFileURI)) + path := strings.TrimSpace(strings.TrimPrefix(l, "ReplyTemplate:")) + // Some are missing .txt + if filepath.Ext(path) == "" { + path += txtFileExt + } + // Some are relative to root, while some are relative to the referencing file. + if dir := filepath.Dir(path); dir == "." { + path = filepath.Join(baseURI, path) + } + var ok bool + path, ok = m.resolveFileReference(path) + if !ok { + debug.Printf("%s: failed to resolve referenced reply template file %q", form.TxtFileURI, path) + continue + } + tmpForm, err := m.buildFormFromTxt(form.ReplyTxtFileURI) + if err != nil { + debug.Printf("%s: failed to load referenced reply template: %v", form.TxtFileURI, err) + } form.ReplyInitialURI = tmpForm.InitialURI form.ReplyViewerURI = tmpForm.ViewerURI } @@ -680,28 +747,27 @@ func findFormFromURI(formName string, folder FormFolder) (Form, error) { } func (m *Manager) findAbsPathForTemplatePath(tmplPath string) (string, error) { - absPathTemplate := filepath.Join(m.config.FormsPath, path.Clean(tmplPath)) - - // now deal with cases where the html file name specified in the .txt file, has different caseness than the actual .html file on disk. - absPathTemplateFolder := filepath.Dir(absPathTemplate) + absPathTemplate := filepath.Join(m.config.FormsPath, filepath.Clean(tmplPath)) - templateDir, err := os.Open(absPathTemplateFolder) - if err != nil { - return "", errors.New("can't read template folder") + // First try to resolve by simply joining the relative path with the absolute path of the templates folder. + if _, err := os.Stat(absPathTemplate); err == nil { + return absPathTemplate, nil } - defer templateDir.Close() - fileNames, err := templateDir.Readdirnames(0) + // Fallback to case-insenstive search. + // Some HTML files references in the .txt files has a different caseness than the actual filename on disk. + absPathTemplateFolder := filepath.Dir(absPathTemplate) + entries, err := os.ReadDir(absPathTemplateFolder) if err != nil { - return "", errors.New("can't read template folder") + return "", err } - - for _, name := range fileNames { + for _, entry := range entries { + name := entry.Name() if strings.EqualFold(filepath.Base(tmplPath), name) { return filepath.Join(absPathTemplateFolder, name), nil } } - return "", fmt.Errorf("unable to resolve absolute template path") + return "", fmt.Errorf("failed to resolve relative path: %s", tmplPath) } // gpsPos returns the current GPS Position @@ -795,8 +861,8 @@ func posToGridSquare(pos gpsd.Position) string { return gridsquare } -func (m *Manager) fillFormTemplate(absPathTemplate string, formDestURL string, placeholderRegEx *regexp.Regexp, formVars map[string]string) (string, error) { - fUnsanitized, err := os.Open(absPathTemplate) +func (m *Manager) fillFormTemplate(tmplPath string, formDestURL string, placeholderRegEx *regexp.Regexp, formVars map[string]string) (string, error) { + fUnsanitized, err := m.open(tmplPath) if err != nil { return "", err } @@ -808,10 +874,10 @@ func (m *Manager) fillFormTemplate(absPathTemplate string, formDestURL string, p sanitizedFileContent, err := io.ReadAll(f) if err != nil { - return "", fmt.Errorf("error reading file %s", absPathTemplate) + return "", fmt.Errorf("error reading file %s", tmplPath) } if !utf8.Valid(sanitizedFileContent) { - log.Printf("Warning: unsupported string encoding in template %s, expected utf-8", absPathTemplate) + log.Printf("Warning: unsupported string encoding in template %s, expected utf-8", tmplPath) } now := time.Now() @@ -909,12 +975,9 @@ type formMessageBuilder struct { // build returns message subject, body, and XML attachment content for the given template and variable map func (b formMessageBuilder) build() (MessageForm, error) { - tmplPath := filepath.Join(b.FormsMgr.config.FormsPath, b.Template.TxtFileURI) - if filepath.Ext(tmplPath) == "" { - tmplPath += txtFileExt - } + tmplPath := b.Template.TxtFileURI if b.IsReply && b.Template.ReplyTxtFileURI != "" { - tmplPath = filepath.Join(b.FormsMgr.config.FormsPath, b.Template.ReplyTxtFileURI) + tmplPath = b.Template.ReplyTxtFileURI } b.initFormValues() @@ -999,7 +1062,7 @@ func (b formMessageBuilder) initFormValues() { } func (b formMessageBuilder) scanTmplBuildMessage(tmplPath string) (MessageForm, error) { - infile, err := os.Open(tmplPath) + infile, err := b.FormsMgr.open(tmplPath) if err != nil { return MessageForm{}, err }