Conversation
Greptile SummaryThis PR adds web app ( Issues found:
Confidence Score: 2/5
Important Files Changed
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| // Read request body | ||
| reqBody, err := io.ReadAll(req.Body) | ||
| if err != nil { | ||
| l.Error().Err(err).Msg("Failed to read request body") | ||
| writeErrorResponse(clientConn, "failed to read request body") | ||
| continue | ||
| } | ||
| req.Body.Close() |
There was a problem hiding this comment.
No request body size limit (potential DoS)
io.ReadAll(req.Body) reads the entire request body into memory with no upper bound. A malicious or misbehaving client could send an extremely large body (e.g., a multi-GB upload) causing memory exhaustion on the gateway process. Consider wrapping the body with http.MaxBytesReader before reading:
| // Read request body | |
| reqBody, err := io.ReadAll(req.Body) | |
| if err != nil { | |
| l.Error().Err(err).Msg("Failed to read request body") | |
| writeErrorResponse(clientConn, "failed to read request body") | |
| continue | |
| } | |
| req.Body.Close() | |
| // Read request body | |
| req.Body = http.MaxBytesReader(nil, req.Body, 32*1024*1024) // 32 MB limit | |
| reqBody, err := io.ReadAll(req.Body) | |
| if err != nil { | |
| l.Error().Err(err).Msg("Failed to read request body") | |
| writeErrorResponse(clientConn, "failed to read request body") | |
| continue | |
| } | |
| req.Body.Close() |
Note: http.MaxBytesReader requires an http.ResponseWriter as its first argument; if that doesn't fit here, you can instead use io.LimitReader:
reqBody, err := io.ReadAll(io.LimitReader(req.Body, 32*1024*1024))| transport := &http.Transport{ | ||
| DisableKeepAlives: false, | ||
| MaxIdleConns: 10, | ||
| IdleConnTimeout: 30 * time.Second, | ||
| TLSClientConfig: p.config.TLSConfig, | ||
| } | ||
| client := &http.Client{ | ||
| Transport: transport, | ||
| Timeout: 30 * time.Second, | ||
| // Don't follow redirects — let the client handle them | ||
| CheckRedirect: func(req *http.Request, via []*http.Request) error { | ||
| return http.ErrUseLastResponse | ||
| }, | ||
| } |
There was a problem hiding this comment.
HTTP transport and client created per request
A new http.Transport and http.Client is instantiated on every iteration of the loop, which means:
- Connection pooling is effectively disabled — a new TCP/TLS handshake is performed for every proxied request
MaxIdleConns: 10andDisableKeepAlives: falsehave no practical effect since the transport is discarded after each request- Under load, this exhausts file descriptors and significantly increases latency
The transport and client should be created once per proxy instance (e.g., in NewWebAppProxy or at the start of HandleConnection) and reused across requests:
// In WebAppProxy struct
type WebAppProxy struct {
config WebAppProxyConfig
transport *http.Transport
client *http.Client
}
func NewWebAppProxy(config WebAppProxyConfig) *WebAppProxy {
transport := &http.Transport{
DisableKeepAlives: false,
MaxIdleConns: 10,
IdleConnTimeout: 30 * time.Second,
TLSClientConfig: config.TLSConfig,
}
client := &http.Client{
Transport: transport,
Timeout: 30 * time.Second,
CheckRedirect: func(req *http.Request, via []*http.Request) error {
return http.ErrUseLastResponse
},
}
return &WebAppProxy{config: config, transport: transport, client: client}
}| func writeErrorResponse(conn net.Conn, message string) { | ||
| errResp := fmt.Sprintf( | ||
| "HTTP/1.1 502 Bad Gateway\r\nContent-Type: application/json\r\n\r\n{\"message\": \"gateway: %s\"}", | ||
| message, | ||
| ) | ||
| conn.Write([]byte(errResp)) | ||
| } |
There was a problem hiding this comment.
JSON injection in error response body
The message argument is interpolated directly into a raw JSON string without escaping. If err.Error() (line 150) returns a string containing double-quotes or backslashes (e.g., a network error mentioning a path like "failed to dial \"target:8080\": ..."), the resulting response body will be malformed JSON.
For example, the call at line 150:
writeErrorResponse(clientConn, fmt.Sprintf("failed to reach target: %s", err.Error()))could produce: {"message": "gateway: failed to reach target: dial tcp: lookup "evil" ..."} — invalid JSON.
Use encoding/json to safely encode the message:
func writeErrorResponse(conn net.Conn, message string) {
type errBody struct {
Message string `json:"message"`
}
body, _ := json.Marshal(errBody{Message: "gateway: " + message})
errResp := fmt.Sprintf(
"HTTP/1.1 502 Bad Gateway\r\nContent-Type: application/json\r\nContent-Length: %d\r\n\r\n%s",
len(body), body,
)
conn.Write([]byte(errResp))
}1 participant
Please see: gilesv/infisical#1