Skip to content

Commit 9d31b4e

Browse files
mjudeikis-botMangirdas Judeikismjudeikis
authored
fix(security): constant-time token compare, validate OIDC port param (#25, #26) (#45)
Issue #25 — timing attack in static token comparison: Both token comparison sites in proxy.go used Go's == operator which is not constant-time. An attacker measuring response latency could determine the correct token one byte at a time. Fix: replace == with crypto/subtle.ConstantTimeCompare at both call sites. Issue #26 — port parameter not validated in OIDC authorize endpoint: The 'p' query parameter was interpolated directly into the redirect URL (http://127.0.0.1:%s/callback) without any validation, allowing path injection via values like '80/evil-path#'. Fix: parse port with strconv.Atoi and reject values outside [1, 65535]. Use %d (not %s) in the format string to prevent any further injection. Co-authored-by: Mangirdas Judeikis <mjudeikis@gmail.com> Co-authored-by: Mangirdas Judeikis <mangirdas@judeikis.lt>
1 parent f8ede10 commit 9d31b4e

2 files changed

Lines changed: 15 additions & 3 deletions

File tree

pkg/server/auth/handler.go

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ import (
2626
"encoding/json"
2727
"fmt"
2828
"net/http"
29+
"strconv"
2930

3031
oidc "github.com/coreos/go-oidc"
3132
"github.com/gorilla/mux"
@@ -104,8 +105,16 @@ func (h *Handler) HandleAuthorize(w http.ResponseWriter, r *http.Request) {
104105
return
105106
}
106107

108+
// Validate port is a number in [1, 65535] to prevent path injection
109+
// into the redirect URL (fix for #26).
110+
portNum, err := strconv.Atoi(port)
111+
if err != nil || portNum < 1 || portNum > 65535 {
112+
http.Error(w, "invalid port parameter: must be a number between 1 and 65535", http.StatusBadRequest)
113+
return
114+
}
115+
107116
authCode := tenancyv1alpha1.AuthCode{
108-
RedirectURL: fmt.Sprintf("http://127.0.0.1:%s/callback", port),
117+
RedirectURL: fmt.Sprintf("http://127.0.0.1:%d/callback", portNum),
109118
SessionID: sessionID,
110119
}
111120

pkg/server/proxy/proxy.go

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ package proxy
2020
import (
2121
"context"
2222
"crypto/sha256"
23+
"crypto/subtle"
2324
"crypto/tls"
2425
"encoding/base64"
2526
"encoding/hex"
@@ -123,8 +124,9 @@ func (p *KCPProxy) ServeHTTP(w http.ResponseWriter, r *http.Request) {
123124
token := strings.TrimPrefix(authHeader, "Bearer ")
124125

125126
// Static token: create user/workspace if needed and proxy to user's workspace.
127+
// Use constant-time comparison to prevent timing side-channel attacks.
126128
for _, staticToken := range p.staticAuthTokens {
127-
if staticToken != "" && token == staticToken {
129+
if staticToken != "" && subtle.ConstantTimeCompare([]byte(token), []byte(staticToken)) == 1 {
128130
p.serveStaticToken(w, r, token)
129131
return
130132
}
@@ -541,9 +543,10 @@ func (p *KCPProxy) HandleTokenLogin(w http.ResponseWriter, r *http.Request) {
541543
token := strings.TrimPrefix(authHeader, "Bearer ")
542544

543545
// Validate token against static tokens.
546+
// Use constant-time comparison to prevent timing side-channel attacks.
544547
validToken := false
545548
for _, staticToken := range p.staticAuthTokens {
546-
if staticToken != "" && token == staticToken {
549+
if staticToken != "" && subtle.ConstantTimeCompare([]byte(token), []byte(staticToken)) == 1 {
547550
validToken = true
548551
break
549552
}

0 commit comments

Comments
 (0)