From a75a20b50be673b96b1d42187b97f8cfe60728df Mon Sep 17 00:00:00 2001 From: Luca Corbatto Date: Fri, 23 Dec 2022 08:03:04 +0100 Subject: [PATCH] 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)), "/") +}