Merge pull request #711 from projectdiscovery/dwisiswant0/fix/httputil/prevent-memory-corruption-in-ResponseChain

fix(httputil): prevent memory corruption in `ResponseChain`

Author: Mzack9999

http/respChain.go

@@ -259,10 +259,9 @@ func (r *ResponseChain) HeadersBytes() []byte {
 
 // HeadersString returns the current response headers as string in the chain.
 //
-// The returned string is valid only until Close() is called.
-// This is a zero-copy operation for performance.
+// The returned string is a copy and remains valid even after Close() is called.
 func (r *ResponseChain) HeadersString() string {
-	return conversion.String(r.headers.Bytes())
+	return r.headers.String()
 }
 
 // Body returns the current response body buffer in the chain.
@@ -282,10 +281,9 @@ func (r *ResponseChain) BodyBytes() []byte {
 
 // BodyString returns the current response body as string in the chain.
 //
-// The returned string is valid only until Close() is called.
-// This is a zero-copy operation for performance.
+// The returned string is a copy and remains valid even after Close() is called.
 func (r *ResponseChain) BodyString() string {
-	return conversion.String(r.body.Bytes())
+	return r.body.String()
 }
 
 // FullResponse returns a new buffer containing headers+body.
@@ -319,7 +317,6 @@ func (r *ResponseChain) FullResponseBytes() []byte {
 // FullResponseString returns the current response as string in the chain.
 //
 // The returned string is a copy and remains valid even after Close() is called.
-// This is a zero-copy operation from the byte slice.
 func (r *ResponseChain) FullResponseString() string {
 	return conversion.String(r.FullResponseBytes())
 }

http/respChain_test.go

@@ -1182,3 +1182,48 @@ func TestLimitedBuffer_Pool(t *testing.T) {
 	require.Equal(t, len(data), buf.Len())
 	require.Equal(t, data, buf.Bytes())
 }
+
+func TestResponseChain_StringSafety(t *testing.T) {
+	bodyContent := "Original Body Content That Should Be Preserved Yeah Okay LOL"
+	headerKey := "X-Safety-Test"
+	headerValue := "Original Header Value"
+
+	resp := &http.Response{
+		StatusCode: 200,
+		Body:       io.NopCloser(strings.NewReader(bodyContent)),
+		Header:     http.Header{headerKey: []string{headerValue}},
+		Proto:      "HTTP/1.1",
+		ProtoMajor: 1,
+		ProtoMinor: 1,
+	}
+
+	rc := NewResponseChain(resp, -1)
+	err := rc.Fill()
+	require.NoError(t, err)
+
+	bodyStr := rc.BodyString()
+	headersStr := rc.HeadersString()
+
+	assert.Equal(t, bodyContent, bodyStr)
+	assert.Contains(t, headersStr, headerValue)
+
+	rc.Close()
+
+	// Now attempt to pollute the pool and overwrite the memory.
+	// We get a bunch of buffers and write garbage to them.
+	var buffers []*bytes.Buffer
+	for i := 0; i < 100; i++ {
+		buf := getBuffer()
+		buffers = append(buffers, buf)
+
+		buf.Reset()
+		buf.WriteString("ALERTA_GARBAGE_DATA_OVERWRITING_MEMORY_ALERTA_GARBAGE_DATA_OVERWRITING_MEMORY")
+	}
+
+	for _, buf := range buffers {
+		putBuffer(buf)
+	}
+
+	assert.Equal(t, bodyContent, bodyStr, "BodyString() content changed after buffer reuse - unsafe memory sharing detected")
+	assert.Contains(t, headersStr, headerValue, "HeadersString() content changed after buffer reuse - unsafe memory sharing detected")
+}