Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CSRF middleware form lookup consumes all the request body #2600

Open
3 tasks done
alessandro-p opened this issue Mar 1, 2024 · 1 comment
Open
3 tasks done

CSRF middleware form lookup consumes all the request body #2600

alessandro-p opened this issue Mar 1, 2024 · 1 comment

Comments

@alessandro-p
Copy link

alessandro-p commented Mar 1, 2024

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

  • Dependencies installed
  • No typos
  • 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/v4@v4.11.4/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.

@aldas
Copy link
Contributor

aldas commented Apr 6, 2024

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

c.Request().Body = io.NopCloser(bytes.NewBuffer(reqBody)) // Reset

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants