Fix CodeQL log injection warning
by removing newlines from logged user input.
This commit is contained in:

committed by
Sebastian Serth

parent
97a2311a74
commit
1e59c1146e
@ -27,7 +27,8 @@ func HTTPAuthenticationMiddleware(next http.Handler) http.Handler {
|
|||||||
return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||||
token := r.Header.Get(TokenHeader)
|
token := r.Header.Get(TokenHeader)
|
||||||
if subtle.ConstantTimeCompare([]byte(token), correctAuthenticationToken) == 0 {
|
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)
|
w.WriteHeader(http.StatusUnauthorized)
|
||||||
return
|
return
|
||||||
}
|
}
|
||||||
|
@ -8,10 +8,12 @@ import (
|
|||||||
"github.com/openHPI/poseidon/internal/config"
|
"github.com/openHPI/poseidon/internal/config"
|
||||||
"github.com/openHPI/poseidon/internal/runner"
|
"github.com/openHPI/poseidon/internal/runner"
|
||||||
"github.com/openHPI/poseidon/pkg/dto"
|
"github.com/openHPI/poseidon/pkg/dto"
|
||||||
|
"github.com/openHPI/poseidon/pkg/logging"
|
||||||
"github.com/openHPI/poseidon/pkg/monitoring"
|
"github.com/openHPI/poseidon/pkg/monitoring"
|
||||||
"io"
|
"io"
|
||||||
"net/http"
|
"net/http"
|
||||||
"net/url"
|
"net/url"
|
||||||
|
"strconv"
|
||||||
)
|
)
|
||||||
|
|
||||||
const (
|
const (
|
||||||
@ -58,7 +60,8 @@ func (r *RunnerController) provide(writer http.ResponseWriter, request *http.Req
|
|||||||
case errors.Is(err, runner.ErrUnknownExecutionEnvironment):
|
case errors.Is(err, runner.ErrUnknownExecutionEnvironment):
|
||||||
writeNotFound(writer, err)
|
writeNotFound(writer, err)
|
||||||
case errors.Is(err, runner.ErrNoRunnersAvailable):
|
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)
|
writeInternalServerError(writer, err, dto.ErrorNomadOverload)
|
||||||
default:
|
default:
|
||||||
writeInternalServerError(writer, err, dto.ErrorUnknown)
|
writeInternalServerError(writer, err, dto.ErrorUnknown)
|
||||||
|
@ -8,6 +8,7 @@ import (
|
|||||||
"github.com/gorilla/websocket"
|
"github.com/gorilla/websocket"
|
||||||
"github.com/openHPI/poseidon/internal/runner"
|
"github.com/openHPI/poseidon/internal/runner"
|
||||||
"github.com/openHPI/poseidon/pkg/dto"
|
"github.com/openHPI/poseidon/pkg/dto"
|
||||||
|
"github.com/openHPI/poseidon/pkg/logging"
|
||||||
"github.com/openHPI/poseidon/pkg/monitoring"
|
"github.com/openHPI/poseidon/pkg/monitoring"
|
||||||
"io"
|
"io"
|
||||||
"net/http"
|
"net/http"
|
||||||
@ -357,7 +358,9 @@ func (r *RunnerController) connectToRunner(writer http.ResponseWriter, request *
|
|||||||
return
|
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)
|
exit, cancel, err := targetRunner.ExecuteInteractively(executionID, proxy.Stdin, proxy.Stdout, proxy.Stderr)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
proxy.closeWithError(fmt.Sprintf("execution failed with: %v", err))
|
proxy.closeWithError(fmt.Sprintf("execution failed with: %v", err))
|
||||||
|
@ -8,6 +8,7 @@ import (
|
|||||||
"net"
|
"net"
|
||||||
"net/http"
|
"net/http"
|
||||||
"os"
|
"os"
|
||||||
|
"strings"
|
||||||
"time"
|
"time"
|
||||||
)
|
)
|
||||||
|
|
||||||
@ -70,7 +71,7 @@ func (writer *loggingResponseWriter) Hijack() (net.Conn, *bufio.ReadWriter, erro
|
|||||||
func HTTPLoggingMiddleware(next http.Handler) http.Handler {
|
func HTTPLoggingMiddleware(next http.Handler) http.Handler {
|
||||||
return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||||
start := time.Now().UTC()
|
start := time.Now().UTC()
|
||||||
path := r.URL.Path
|
path := RemoveNewlineSymbol(r.URL.Path)
|
||||||
|
|
||||||
lrw := NewLoggingResponseWriter(w)
|
lrw := NewLoggingResponseWriter(w)
|
||||||
next.ServeHTTP(lrw, r)
|
next.ServeHTTP(lrw, r)
|
||||||
@ -81,7 +82,7 @@ func HTTPLoggingMiddleware(next http.Handler) http.Handler {
|
|||||||
"method": r.Method,
|
"method": r.Method,
|
||||||
"path": path,
|
"path": path,
|
||||||
"duration": latency,
|
"duration": latency,
|
||||||
"user_agent": r.UserAgent(),
|
"user_agent": RemoveNewlineSymbol(r.UserAgent()),
|
||||||
})
|
})
|
||||||
if lrw.StatusCode >= http.StatusInternalServerError {
|
if lrw.StatusCode >= http.StatusInternalServerError {
|
||||||
logEntry.Error("Failing " + path)
|
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
|
||||||
|
}
|
||||||
|
Reference in New Issue
Block a user