From fef9a33ceb66f6b929839f7eaf393b629681bc5d Mon Sep 17 00:00:00 2001 From: Luca Corbatto Date: Fri, 23 Dec 2022 07:35:35 +0100 Subject: [PATCH 1/4] Fixes path injection vulnerability #35 --- archiver/remoteServer.go | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/archiver/remoteServer.go b/archiver/remoteServer.go index daaee34..2547b6e 100644 --- a/archiver/remoteServer.go +++ b/archiver/remoteServer.go @@ -11,6 +11,7 @@ import ( "math/rand" "net/http" "os" + "path" "path/filepath" "strings" "sync" @@ -241,14 +242,24 @@ type CreateReportRequest struct { Name string `json:"name"` } +func sanitizeFilename(name string) string { + return path.Base(path.Clean("/" + name)) +} + func (s *ArchiverServer) createReport(c *gin.Context) { var req CreateReportRequest if err := c.ShouldBindJSON(&req); handleError(c, err) { return } + reportName := sanitizeFilename(req.Name) + if reportName == "." || reportName == "/" { + handleError(c, fmt.Errorf("invalid report name '%s'", req.Name)) + return + } + reportID := generateReportID() - _, err := s.registerReport(reportID, req.Name) + _, err := s.registerReport(reportID, reportName) if handleError(c, err) { return } From a75a20b50be673b96b1d42187b97f8cfe60728df Mon Sep 17 00:00:00 2001 From: Luca Corbatto Date: Fri, 23 Dec 2022 08:03:04 +0100 Subject: [PATCH 2/4] Fixes various log injection vulnerabilities #35 --- archiver/remoteServer.go | 30 +++++++++++++++++++----------- 1 file changed, 19 insertions(+), 11 deletions(-) diff --git a/archiver/remoteServer.go b/archiver/remoteServer.go index 2547b6e..43e6ae3 100644 --- a/archiver/remoteServer.go +++ b/archiver/remoteServer.go @@ -202,7 +202,7 @@ func (s *ArchiverServer) registerReport(reportID, reportName string) (*reportHan _, exists := s.openReports[reportID] if exists { - return nil, fmt.Errorf("report with ID '%s' already exists", reportID) + return nil, fmt.Errorf("report with ID '%s' already exists", sanitizeStringForLogs(reportID)) } handler, err := newReportHandler(s.outdir, reportName, s.outerExt, s.wcBuilder) @@ -220,7 +220,7 @@ func (s *ArchiverServer) getReport(reportID string) (*reportHandler, error) { report, exists := s.openReports[reportID] if !exists { - return nil, fmt.Errorf("report with ID '%s' does not exists", reportID) + return nil, fmt.Errorf("report with ID '%s' does not exists", sanitizeStringForLogs(reportID)) } return report, nil } @@ -231,7 +231,7 @@ func (s *ArchiverServer) getAndRemoveReport(reportID string) (*reportHandler, er report, exists := s.openReports[reportID] if !exists { - return nil, fmt.Errorf("report with ID '%s' does not exists", reportID) + return nil, fmt.Errorf("report with ID '%s' does not exists", sanitizeStringForLogs(reportID)) } delete(s.openReports, reportID) @@ -243,7 +243,7 @@ type CreateReportRequest struct { } func sanitizeFilename(name string) string { - return path.Base(path.Clean("/" + name)) + return path.Base(path.Clean("/" + removeNewlines(name))) } func (s *ArchiverServer) createReport(c *gin.Context) { @@ -254,7 +254,7 @@ func (s *ArchiverServer) createReport(c *gin.Context) { reportName := sanitizeFilename(req.Name) if reportName == "." || reportName == "/" { - handleError(c, fmt.Errorf("invalid report name '%s'", req.Name)) + handleError(c, fmt.Errorf("invalid report name '%s'", sanitizeStringForLogs(req.Name))) return } @@ -264,7 +264,7 @@ func (s *ArchiverServer) createReport(c *gin.Context) { return } - logrus.Infof("Creating new report '%s' with ID '%s'", req.Name, reportID) + logrus.Infof("Creating new report '%s' with ID '%s'", sanitizeStringForLogs(req.Name), reportID) c.JSON(http.StatusOK, gin.H{ "error": nil, @@ -280,7 +280,7 @@ func (s *ArchiverServer) closeReport(c *gin.Context) { if handleError(c, report.Close()) { return } - logrus.Infof("Closed report with ID '%s'", c.Param("report")) + logrus.Infof("Closed report with ID '%s'", sanitizeStringForLogs(c.Param("report"))) sendOkay(c) } @@ -368,10 +368,6 @@ func newReportHandler(dir, reportName, outerExt string, wcBuilder WriteCloserBui }, nil } -func sanitizePath(path string) string { - return strings.Trim(filepath.Clean(path), "/") -} - func (h *reportHandler) CreateFile(filepath string) error { h.openFilesMux.Lock() defer h.openFilesMux.Unlock() @@ -434,3 +430,15 @@ func (h *reportHandler) Close() error { err = errors.NewMultiError(err, os.Rename(h.reportArchiveSwpPath, h.reportArchivePath)) return err } + +func removeNewlines(s string) string { + return strings.Replace(strings.Replace(s, "\n", "", -1), "\r", "", -1) +} + +func sanitizeStringForLogs(s string) string { + return removeNewlines(s) +} + +func sanitizePath(path string) string { + return strings.Trim(filepath.Clean(removeNewlines(path)), "/") +} From 3998514759056fbaf7f59f7ab1355f482750e008 Mon Sep 17 00:00:00 2001 From: Luca Corbatto Date: Fri, 23 Dec 2022 08:03:24 +0100 Subject: [PATCH 3/4] Fixes silent startup failures --- app/receive.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/app/receive.go b/app/receive.go index d9e0d0c..fb76689 100644 --- a/app/receive.go +++ b/app/receive.go @@ -98,14 +98,14 @@ func receive(c *cli.Context) error { }() err = reportServer.Start() + if err != http.ErrServerClosed { + return cli.Exit(err, 10) + } switch { case <-shutdownStarted: <-shutdownCompleted default: } - if err != http.ErrServerClosed { - return cli.Exit(err, 10) - } return nil } From 68d27354df5b4a1315f70933e9d5143ac4cc6a86 Mon Sep 17 00:00:00 2001 From: Luca Corbatto Date: Fri, 23 Dec 2022 08:21:45 +0100 Subject: [PATCH 4/4] Bumps version --- version/version.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/version/version.go b/version/version.go index 4a4c3df..dd0ed47 100644 --- a/version/version.go +++ b/version/version.go @@ -10,7 +10,7 @@ import ( var YapscanVersion = Version{ Major: 0, Minor: 19, - Bugfix: 0, + Bugfix: 1, } type Version struct {