Skip to content

Conversation

@aldas
Copy link
Contributor

@aldas aldas commented Mar 15, 2022

Fix for #2135 CSRF middleware not being able to extract token from multipart/form-data form

This can be tested by hand:

func main() { e := echo.New() e.Use(middleware.Logger()) e.Use(middleware.Recover()) e.Use(middleware.CSRFWithConfig(middleware.CSRFConfig{ TokenLookup: "form:csrf",	})) e.POST("/form", func(c echo.Context) error { csrf, ok := c.Get("csrf").(string) if !ok { return echo.NewHTTPError(http.StatusBadRequest, "missing CSRF value")	} return c.String(http.StatusCreated, csrf)	}) if err := e.Start(":8080"); err != http.ErrServerClosed { log.Fatal(err)	} }
echo "<div>hi</div>" > test.html curl --trace-ascii /dev/stdout \ --cookie "_csrf=test" \ -F csrf=test \ -F upload=@test.html \ http://localhost:8080/form

output of curl after fix

x@x:~/code/echo/main$ curl --trace-ascii /dev/stdout --cookie "_csrf=test" -F csrf=test -F upload=@test.html http://localhost:8080/form == Info: Trying 127.0.0.1:8080... == Info: Connected to localhost (127.0.0.1) port 8080 (#0) => Send header, 210 bytes (0xd2) 0000: POST /form HTTP/1.1 0015: Host: localhost:8080 002b: User-Agent: curl/7.74.0 0044: Accept: */* 0051: Cookie: _csrf=test 0065: Content-Length: 299 007a: Content-Type: multipart/form-data; boundary=-------------------- 00ba: ----a297ba70c335a0d7 00d0: => Send data, 299 bytes (0x12b) 0000: --------------------------a297ba70c335a0d7 002c: Content-Disposition: form-data; name="csrf" 0059: 005b: test 0061: --------------------------a297ba70c335a0d7 008d: Content-Disposition: form-data; name="upload"; filename="test.ht 00cd: ml" 00d2: Content-Type: text/html 00eb: 00ed: <div>hi</div>. 00fd: --------------------------a297ba70c335a0d7-- == Info: We are completely uploaded and fine == Info: Mark bundle as not supporting multiuse <= Recv header, 22 bytes (0x16) 0000: HTTP/1.1 201 Created <= Recv header, 41 bytes (0x29) 0000: Content-Type: text/plain; charset=UTF-8 <= Recv header, 63 bytes (0x3f) 0000: Set-Cookie: _csrf=test; Expires=Wed, 16 Mar 2022 19:00:31 GMT <= Recv header, 14 bytes (0xe) 0000: Vary: Cookie <= Recv header, 37 bytes (0x25) 0000: Date: Tue, 15 Mar 2022 19:00:31 GMT <= Recv header, 19 bytes (0x13) 0000: Content-Length: 4 <= Recv header, 2 bytes (0x2) 0000: <= Recv data, 4 bytes (0x4) 0000: test == Info: Connection #0 to host localhost left intact test
@codecov
Copy link

codecov bot commented Mar 15, 2022

Codecov Report

Merging #2136 (283d1ac) into master (b445958) will not change coverage.
The diff coverage is 100.00%.

@@ Coverage Diff @@ ## master #2136 +/- ## ======================================= Coverage 92.10% 92.10% ======================================= Files 37 37 Lines 3028 3028 ======================================= Hits 2789 2789 Misses 150 150 Partials 89 89 
Impacted Files Coverage Δ
middleware/extractor.go 98.83% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b445958...283d1ac. Read the comment docs.

@aldas aldas requested a review from lammel March 15, 2022 19:04
@aldas
Copy link
Contributor Author

aldas commented Mar 15, 2022

@lammel please review

Copy link
Contributor

@lammel lammel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@lammel lammel merged commit 01d7d01 into labstack:master Mar 15, 2022
@aldas aldas deleted the fix_csrf_multipartform_values branch July 12, 2022 19:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

2 participants