Repository: labstack/echo (go)

Issue:
c.bind() binds both POST request body and query to given struct causing security vulnerability and problems with slices
### Issue Description

Commit https://github.com/labstack/echo/commit/b129098169e182073d0a60e20cbed143b183e765 removed content length and request method check from c.bind() function. This causes many unwanted side effects

1. security vulnerability - now it is possible to inject field values from query param to POST method body with bind.  This could allow attacker to inject to fields that are not meant to be filled from bind.  I know that you should not pass binded values directly to services etc but before this change this was not possible with query params. It is way easier to manipulate query params than request body.
 
see example:
```go
func TestSecurityRisk(t *testing.T) {
    type User struct {
        ID int `json:"id"`
        IsAdmin bool // not 'exposed' value should not be bind DEFINITELY from URL/ROUTE parameters
    }

    e := echo.New()
    req := httptest.NewRequest(http.MethodPost, "/api/endpoint?IsAdmin=true", strings.NewReader(`{"id": 1}`))
    req.Header.Set(echo.HeaderContentType, echo.MIMEApplicationJSON)
    rec := httptest.NewRecorder()
    c := e.NewContext(req, rec)

    err := func(c echo.Context) error {
        var payload User
        if err := c.Bind(&payload); err != nil {
            return c.JSON(http.StatusBadRequest, echo.Map{"error": err})
        }

        // passing bind value directly to service is bad practice and could lead to security risks but c.bind()
        // doing it implicitly is also very bad (even worse)

        // err := service.addUser(c.Request().Context(), user)

        if payload.IsAdmin {
            panic("passing bind value directly to service is bad practice and could lead to security risks")
        }

        return c.JSON(http.StatusOK, payload)
    }(c)
    if err != nil {
        t.Fatal(err)
    }
}
```

2. if request contains query param + body and we are binding to slice request fails with `binding element must be a struct`

Example:
```go
func TestBindingToSliceWithQueryParam(t *testing.T) {
	type User struct {
		ID int `json:"id"`
	}

	e := echo.New()
	req := httptest.NewRequest(http.MethodPost, "/api/endpoint?lang=et", strings.NewReader(`[{"id": 1}]`))
	req.Header.Set(echo.HeaderContentType, echo.MIMEApplicationJSON)
	rec := httptest.NewRecorder()
	c := e.NewContext(req, rec)

	err := func(c echo.Context) error {
		var payload []User
		if err := c.Bind(&payload); err != nil {
			return c.JSON(http.StatusBadRequest, echo.Map{"error": err})
		}

		return c.JSON(http.StatusOK, payload)
	}(c)
	if err != nil {
		t.Fatal(err)
	}
	response := string(rec.Body.Bytes())
	if strings.Contains(response, "binding element must be a struct"){
		t.Fatal(response)
	}
}
```

### Expected behaviour

I think expected behavior is as it was before https://github.com/labstack/echo/commit/b129098169e182073d0a60e20cbed143b183e765


### Version/commit

v4.1.17

Pull request:
Bind query params only for HTTP GET/DELETE methods
fixes #1670 and continues https://github.com/labstack/echo/pull/1681#issuecomment-746705169 proposed changes 

DefaultBinder.Bind() binds now query params to struct only for GET and DELETE methods. For POST/PUT/PATCH query params are not considered anymore. This restores pre [v4.1.11](https://github.com/labstack/echo/commit/b129098169e182073d0a60e20cbed143b183e765) behavior. 

Patch:
diff --git a/Makefile b/Makefile
index dfcb6c0..c369913 100644
--- a/Makefile
+++ b/Makefile
@@ -1,3 +1,27 @@
+PKG := "github.com/labstack/echo"
+PKG_LIST := $(shell go list ${PKG}/...)
+
 tag:
 	@git tag `grep -P '^\tversion = ' echo.go|cut -f2 -d'"'`
 	@git tag|grep -v ^v
+
+.DEFAULT_GOAL := check
+check: lint vet race ## Check project
+
+init:
+	@go get -u golang.org/x/lint/golint
+
+lint: ## Lint the files
+	@golint -set_exit_status ${PKG_LIST}
+
+vet: ## Vet the files
+	@go vet ${PKG_LIST}
+
+test: ## Run tests
+	@go test -short ${PKG_LIST}
+
+race: ## Run tests with data race detector
+	@go test -race ${PKG_LIST}
+
+help: ## Display this help screen
+	@grep -h -E '^[a-zA-Z_-]+:.*?## .*$$' $(MAKEFILE_LIST) | awk 'BEGIN {FS = ":.*?## "}; {printf "\033[36m%-30s\033[0m %s\n", $$1, $$2}'
diff --git a/bind.go b/bind.go
index c7be242..acd2bed 100644
--- a/bind.go
+++ b/bind.go
@@ -98,12 +98,20 @@ func (b *DefaultBinder) BindBody(c Context, i interface{}) (err error) {
 }
 
 // Bind implements the `Binder#Bind` function.
+// Binding is done in following order: 1) path params; 2) query params; 3) request body. Each step COULD override previous
+// step binded values. For single source binding use their own methods BindBody, BindQueryParams, BindPathParams.
 func (b *DefaultBinder) Bind(i interface{}, c Context) (err error) {
 	if err := b.BindPathParams(c, i); err != nil {
 		return err
 	}
-	if err = b.BindQueryParams(c, i); err != nil {
-		return err
+	// Issue #1670 - Query params are binded only for GET/DELETE and NOT for usual request with body (POST/PUT/PATCH)
+	// Reasoning here is that parameters in query and bind destination struct could have UNEXPECTED matches and results due that.
+	// i.e. is `&id=1&lang=en` from URL same as `{"id":100,"lang":"de"}` request body and which one should have priority when binding.
+	// This HTTP method check restores pre v4.1.11 behavior and avoids different problems when query is mixed with body
+	if c.Request().Method == http.MethodGet || c.Request().Method == http.MethodDelete {
+		if err = b.BindQueryParams(c, i); err != nil {
+			return err
+		}
 	}
 	return b.BindBody(c, i)
 }


Test patch:
diff --git a/bind_test.go b/bind_test.go
index 60c2f9e..345fbdf 100644
--- a/bind_test.go
+++ b/bind_test.go
@@ -559,7 +559,7 @@ func TestDefaultBinder_BindToStructFromMixedSources(t *testing.T) {
 	// binding is done in steps and one source could overwrite previous source binded data
 	// these tests are to document this behaviour and detect further possible regressions when bind implementation is changed
 
-	type Node struct {
+	type Opts struct {
 		ID   int    `json:"id"`
 		Node string `json:"node"`
 	}
@@ -575,41 +575,77 @@ func TestDefaultBinder_BindToStructFromMixedSources(t *testing.T) {
 		expectError      string
 	}{
 		{
-			name:         "ok, POST bind to struct with: path param + query param + empty body",
+			name:         "ok, POST bind to struct with: path param + query param + body",
 			givenMethod:  http.MethodPost,
 			givenURL:     "/api/real_node/endpoint?node=xxx",
 			givenContent: strings.NewReader(`{"id": 1}`),
-			expect:       &Node{ID: 1, Node: "xxx"}, // in current implementation query params has higher priority than path params
+			expect:       &Opts{ID: 1, Node: "node_from_path"}, // query params are not used, node is filled from path
+		},
+		{
+			name:         "ok, PUT bind to struct with: path param + query param + body",
+			givenMethod:  http.MethodPut,
+			givenURL:     "/api/real_node/endpoint?node=xxx",
+			givenContent: strings.NewReader(`{"id": 1}`),
+			expect:       &Opts{ID: 1, Node: "node_from_path"}, // query params are not used
+		},
+		{
+			name:         "ok, GET bind to struct with: path param + query param + body",
+			givenMethod:  http.MethodGet,
+			givenURL:     "/api/real_node/endpoint?node=xxx",
+			givenContent: strings.NewReader(`{"id": 1}`),
+			expect:       &Opts{ID: 1, Node: "xxx"}, // query overwrites previous path value
+		},
+		{
+			name:         "ok, GET bind to struct with: path param + query param + body",
+			givenMethod:  http.MethodGet,
+			givenURL:     "/api/real_node/endpoint?node=xxx",
+			givenContent: strings.NewReader(`{"id": 1, "node": "zzz"}`),
+			expect:       &Opts{ID: 1, Node: "zzz"}, // body is binded last and overwrites previous (path,query) values
+		},
+		{
+			name:         "ok, DELETE bind to struct with: path param + query param + body",
+			givenMethod:  http.MethodDelete,
+			givenURL:     "/api/real_node/endpoint?node=xxx",
+			givenContent: strings.NewReader(`{"id": 1, "node": "zzz"}`),
+			expect:       &Opts{ID: 1, Node: "zzz"}, // for DELETE body is binded after query params
 		},
 		{
-			name:         "ok, POST bind to struct with: path param + empty body",
+			name:         "ok, POST bind to struct with: path param + body",
 			givenMethod:  http.MethodPost,
 			givenURL:     "/api/real_node/endpoint",
 			givenContent: strings.NewReader(`{"id": 1}`),
-			expect:       &Node{ID: 1, Node: "real_node"},
+			expect:       &Opts{ID: 1, Node: "node_from_path"},
 		},
 		{
 			name:         "ok, POST bind to struct with path + query + body = body has priority",
 			givenMethod:  http.MethodPost,
 			givenURL:     "/api/real_node/endpoint?node=xxx",
 			givenContent: strings.NewReader(`{"id": 1, "node": "zzz"}`),
-			expect:       &Node{ID: 1, Node: "zzz"}, // field value from content has higher priority
+			expect:       &Opts{ID: 1, Node: "zzz"}, // field value from content has higher priority
 		},
 		{
 			name:         "nok, POST body bind failure",
 			givenMethod:  http.MethodPost,
 			givenURL:     "/api/real_node/endpoint?node=xxx",
 			givenContent: strings.NewReader(`{`),
-			expect:       &Node{ID: 0, Node: "xxx"}, // query binding has already modified bind target
+			expect:       &Opts{ID: 0, Node: "node_from_path"}, // query binding has already modified bind target
 			expectError:  "code=400, message=unexpected EOF, internal=unexpected EOF",
 		},
+		{
+			name:         "nok, GET with body bind failure when types are not convertible",
+			givenMethod:  http.MethodGet,
+			givenURL:     "/api/real_node/endpoint?id=nope",
+			givenContent: strings.NewReader(`{"id": 1, "node": "zzz"}`),
+			expect:       &Opts{ID: 0, Node: "node_from_path"}, // path params binding has already modified bind target
+			expectError:  "code=400, message=strconv.ParseInt: parsing \"nope\": invalid syntax, internal=strconv.ParseInt: parsing \"nope\": invalid syntax",
+		},
 		{
 			name:         "nok, GET body bind failure - trying to bind json array to struct",
 			givenMethod:  http.MethodGet,
 			givenURL:     "/api/real_node/endpoint?node=xxx",
 			givenContent: strings.NewReader(`[{"id": 1}]`),
-			expect:       &Node{ID: 0, Node: "xxx"}, // query binding has already modified bind target
-			expectError:  "code=400, message=Unmarshal type error: expected=echo.Node, got=array, field=, offset=1, internal=json: cannot unmarshal array into Go value of type echo.Node",
+			expect:       &Opts{ID: 0, Node: "xxx"}, // query binding has already modified bind target
+			expectError:  "code=400, message=Unmarshal type error: expected=echo.Opts, got=array, field=, offset=1, internal=json: cannot unmarshal array into Go value of type echo.Opts",
 		},
 		{ // binding query params interferes with body. b.BindBody() should be used to bind only body to slice
 			name:             "nok, GET query params bind failure - trying to bind json array to slice",
@@ -617,17 +653,27 @@ func TestDefaultBinder_BindToStructFromMixedSources(t *testing.T) {
 			givenURL:         "/api/real_node/endpoint?node=xxx",
 			givenContent:     strings.NewReader(`[{"id": 1}]`),
 			whenNoPathParams: true,
-			whenBindTarget:   &[]Node{},
-			expect:           &[]Node{},
+			whenBindTarget:   &[]Opts{},
+			expect:           &[]Opts{},
 			expectError:      "code=400, message=binding element must be a struct, internal=binding element must be a struct",
 		},
+		{ // binding query params interferes with body. b.BindBody() should be used to bind only body to slice
+			name:             "ok, POST binding to slice should not be affected query params types",
+			givenMethod:      http.MethodPost,
+			givenURL:         "/api/real_node/endpoint?id=nope&node=xxx",
+			givenContent:     strings.NewReader(`[{"id": 1}]`),
+			whenNoPathParams: true,
+			whenBindTarget:   &[]Opts{},
+			expect:           &[]Opts{{ID: 1}},
+			expectError:      "",
+		},
 		{ // binding path params interferes with body. b.BindBody() should be used to bind only body to slice
 			name:           "nok, GET path params bind failure - trying to bind json array to slice",
 			givenMethod:    http.MethodGet,
 			givenURL:       "/api/real_node/endpoint?node=xxx",
 			givenContent:   strings.NewReader(`[{"id": 1}]`),
-			whenBindTarget: &[]Node{},
-			expect:         &[]Node{},
+			whenBindTarget: &[]Opts{},
+			expect:         &[]Opts{},
 			expectError:    "code=400, message=binding element must be a struct, internal=binding element must be a struct",
 		},
 		{
@@ -636,8 +682,8 @@ func TestDefaultBinder_BindToStructFromMixedSources(t *testing.T) {
 			givenURL:         "/api/real_node/endpoint",
 			givenContent:     strings.NewReader(`[{"id": 1}]`),
 			whenNoPathParams: true,
-			whenBindTarget:   &[]Node{},
-			expect:           &[]Node{{ID: 1, Node: ""}},
+			whenBindTarget:   &[]Opts{},
+			expect:           &[]Opts{{ID: 1, Node: ""}},
 			expectError:      "",
 		},
 	}
@@ -653,14 +699,14 @@ func TestDefaultBinder_BindToStructFromMixedSources(t *testing.T) {
 
 			if !tc.whenNoPathParams {
 				c.SetParamNames("node")
-				c.SetParamValues("real_node")
+				c.SetParamValues("node_from_path")
 			}
 
 			var bindTarget interface{}
 			if tc.whenBindTarget != nil {
 				bindTarget = tc.whenBindTarget
 			} else {
-				bindTarget = &Node{}
+				bindTarget = &Opts{}
 			}
 			b := new(DefaultBinder)