echo icon indicating copy to clipboard operation
echo copied to clipboard

CSRF middleware form lookup consumes all the request body

Open alessandro-p opened this issue 1 year ago • 1 comments

Issue Description

Echo's CORS middleware when TokenLookup is set to form:<your-input-name> consumes all the request body making impossible for downstream operations to use it.

Checklist

  • [x] Dependencies installed
  • [x] No typos
  • [x] Searched existing issues and docs

Expected behaviour

When using TokenLookup to inspect formData to find csrf token it should be possible to reuse the request body. For example, forward the request to a downstream service that will be able to use it.

Actual behaviour

When using TokenLookup to inspect formData, body is completely consumed. This might introduce issues when proxying a request.

Steps to reproduce

See working code below for a full example to reproduce the error

Working code to debug

package main

import (
	"io"
	"net/http"

	"github.com/labstack/echo/v4"
	"github.com/labstack/echo/v4/middleware"
)

func main() {
	e := echo.New()

	e.Use(NewCSRFMiddleware())

	e.POST("/csrf-example", PostExample)

	e.Logger.Fatal(e.Start(":1323"))
}

func NewCSRFMiddleware() echo.MiddlewareFunc {
	return middleware.CSRFWithConfig(middleware.CSRFConfig{ //nolint:exhaustruct
		TokenLookup:    "form:csrf",
		ContextKey:     "csrf",
		CookieName:     "__csrf",
		CookiePath:     "/",
		CookieHTTPOnly: true,
		CookieSecure:   true,
	})
}

func PostExample(c echo.Context) error {
	body, err := io.ReadAll(c.Request().Body)
	if err != nil {
		return c.String(http.StatusInternalServerError, "Error reading request body")
	}
	bodyStr := string(body)

	return c.JSON(http.StatusOK, struct {
		ContentLength int    `json:"content_length"`
		Body          string `json:"body"`
	}{
		ContentLength: int(c.Request().ContentLength),
		Body:          bodyStr,
	})
}

After running the server, simply invoke the route with curl:

curl --location 'http://localhost:1323/csrf-example?=' \
--header 'Cookie: __csrf=test-token' \
--form 'csrf="test-token"'

Result is:

{"content_length":149,"body":""}

Version/commit

echo version: v4.11.4

Additional Debug already done

It seems the issue is in the github.com/labstack/echo/[email protected]/middleware/extractor.go in the function valuesFromForm:

// valuesFromForm returns a function that extracts values from the form field.
func valuesFromForm(name string) ValuesExtractor {
	return func(c echo.Context) ([]string, error) {
		if c.Request().Form == nil {
			_ = c.Request().ParseMultipartForm(32 << 20) // same what `c.Request().FormValue(name)` does
		}
        ....

It seems in fact that the line: c.Request().ParseMultipartForm(32 << 20) is consuming all the body. One workaround that seems to fix the issue is copying the body and restoring after it has been consumed.

alessandro-p avatar Mar 01 '24 08:03 alessandro-p

This is working as intended. When Go standard library parses the Form the request body will be read till the end and can not be read anymore. All form values are stored now in Request.Form (c.Request().Form).

Assuming you are expecting Form in you handler you should access Request.Form* methods and fields. If you need the body in some middleware and it should come before CSRF middleware and use buffering etc strategies so body could be read more than once.

See how bodydump middleware does it https://github.com/labstack/echo/blob/447c92d842e2f91c07eb032f619bea212beeee60/middleware/body_dump.go#L72

aldas avatar Apr 06 '24 13:04 aldas

Thank you, I will look into it :)

alessandro-p avatar Jun 04 '24 09:06 alessandro-p