Skip to content

Commit

Permalink
Fixes various log injection vulnerabilities #35
Browse files Browse the repository at this point in the history
  • Loading branch information
targodan committed Dec 23, 2022
1 parent fef9a33 commit a75a20b
Showing 1 changed file with 19 additions and 11 deletions.
30 changes: 19 additions & 11 deletions archiver/remoteServer.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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
}
Expand All @@ -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)

Expand All @@ -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) {
Expand All @@ -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
}

Expand All @@ -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,
Expand All @@ -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)
}

Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -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)), "/")
}

0 comments on commit a75a20b

Please sign in to comment.