From 1e59c1146e2a7afb229ef276188d7eb6bcfa4dc2 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Maximilian=20Pa=C3=9F?=
 <22845248+mpass99@users.noreply.github.com>
Date: Wed, 1 Jun 2022 18:21:41 +0200
Subject: [PATCH] Fix CodeQL log injection warning

by removing newlines from logged user input.
---
 internal/api/auth/auth.go |  3 ++-
 internal/api/runners.go   |  5 ++++-
 internal/api/websocket.go |  5 ++++-
 pkg/logging/logging.go    | 12 ++++++++++--
 4 files changed, 20 insertions(+), 5 deletions(-)

diff --git a/internal/api/auth/auth.go b/internal/api/auth/auth.go
index cf7d357..f0a6d89 100644
--- a/internal/api/auth/auth.go
+++ b/internal/api/auth/auth.go
@@ -27,7 +27,8 @@ func HTTPAuthenticationMiddleware(next http.Handler) http.Handler {
 	return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 		token := r.Header.Get(TokenHeader)
 		if subtle.ConstantTimeCompare([]byte(token), correctAuthenticationToken) == 0 {
-			log.WithField("token", token).Warn("Incorrect token")
+			log.WithField("token", logging.RemoveNewlineSymbol(token)).
+				Warn("Incorrect token")
 			w.WriteHeader(http.StatusUnauthorized)
 			return
 		}
diff --git a/internal/api/runners.go b/internal/api/runners.go
index 2ce7d1a..ae3fc23 100644
--- a/internal/api/runners.go
+++ b/internal/api/runners.go
@@ -8,10 +8,12 @@ import (
 	"github.com/openHPI/poseidon/internal/config"
 	"github.com/openHPI/poseidon/internal/runner"
 	"github.com/openHPI/poseidon/pkg/dto"
+	"github.com/openHPI/poseidon/pkg/logging"
 	"github.com/openHPI/poseidon/pkg/monitoring"
 	"io"
 	"net/http"
 	"net/url"
+	"strconv"
 )
 
 const (
@@ -58,7 +60,8 @@ func (r *RunnerController) provide(writer http.ResponseWriter, request *http.Req
 		case errors.Is(err, runner.ErrUnknownExecutionEnvironment):
 			writeNotFound(writer, err)
 		case errors.Is(err, runner.ErrNoRunnersAvailable):
-			log.WithField("environment", environmentID).Warn("No runners available")
+			log.WithField("environment", logging.RemoveNewlineSymbol(strconv.Itoa(int(environmentID)))).
+				Warn("No runners available")
 			writeInternalServerError(writer, err, dto.ErrorNomadOverload)
 		default:
 			writeInternalServerError(writer, err, dto.ErrorUnknown)
diff --git a/internal/api/websocket.go b/internal/api/websocket.go
index cf77e45..be14619 100644
--- a/internal/api/websocket.go
+++ b/internal/api/websocket.go
@@ -8,6 +8,7 @@ import (
 	"github.com/gorilla/websocket"
 	"github.com/openHPI/poseidon/internal/runner"
 	"github.com/openHPI/poseidon/pkg/dto"
+	"github.com/openHPI/poseidon/pkg/logging"
 	"github.com/openHPI/poseidon/pkg/monitoring"
 	"io"
 	"net/http"
@@ -357,7 +358,9 @@ func (r *RunnerController) connectToRunner(writer http.ResponseWriter, request *
 		return
 	}
 
-	log.WithField("runnerId", targetRunner.ID()).WithField("executionID", executionID).Info("Running execution")
+	log.WithField("runnerId", targetRunner.ID()).
+		WithField("executionID", logging.RemoveNewlineSymbol(executionID)).
+		Info("Running execution")
 	exit, cancel, err := targetRunner.ExecuteInteractively(executionID, proxy.Stdin, proxy.Stdout, proxy.Stderr)
 	if err != nil {
 		proxy.closeWithError(fmt.Sprintf("execution failed with: %v", err))
diff --git a/pkg/logging/logging.go b/pkg/logging/logging.go
index e2e2470..792f6e5 100644
--- a/pkg/logging/logging.go
+++ b/pkg/logging/logging.go
@@ -8,6 +8,7 @@ import (
 	"net"
 	"net/http"
 	"os"
+	"strings"
 	"time"
 )
 
@@ -70,7 +71,7 @@ func (writer *loggingResponseWriter) Hijack() (net.Conn, *bufio.ReadWriter, erro
 func HTTPLoggingMiddleware(next http.Handler) http.Handler {
 	return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 		start := time.Now().UTC()
-		path := r.URL.Path
+		path := RemoveNewlineSymbol(r.URL.Path)
 
 		lrw := NewLoggingResponseWriter(w)
 		next.ServeHTTP(lrw, r)
@@ -81,7 +82,7 @@ func HTTPLoggingMiddleware(next http.Handler) http.Handler {
 			"method":     r.Method,
 			"path":       path,
 			"duration":   latency,
-			"user_agent": r.UserAgent(),
+			"user_agent": RemoveNewlineSymbol(r.UserAgent()),
 		})
 		if lrw.StatusCode >= http.StatusInternalServerError {
 			logEntry.Error("Failing " + path)
@@ -90,3 +91,10 @@ func HTTPLoggingMiddleware(next http.Handler) http.Handler {
 		}
 	})
 }
+
+// RemoveNewlineSymbol GOOD: remove newlines from user controlled input before logging
+func RemoveNewlineSymbol(data string) string {
+	data = strings.ReplaceAll(data, "\r", "")
+	data = strings.ReplaceAll(data, "\n", "")
+	return data
+}