gin-gonic / gin

Gin is a HTTP web framework written in Go (Golang). It features a Martini-like API with much better performance -- up to 40 times faster. If you need smashing performance, get yourself some Gin.
https://gin-gonic.com/
MIT License
78.96k stars 8.02k forks source link

Wildcard parameter incorrectly decodes URIs recursively #2047

Open john8329 opened 5 years ago

john8329 commented 5 years ago

Description

I have a route defined like this:

appRt.GET("/*request", s.AppDispatchRequest)

The wildcard request parameter should get the full path after the /, and I extract it like this:

uri := c.Param("request")

The issue I'm having is that if I pass a path with some URL Encoded slashes (%2F), they get recursively decoded. So if for example I request an URI like /run/cmd/S123L%2FA, then the value of uri is /run/cmd/S123L/A, which is incorrect, as I don't expect the slashes to be decoded. This could be a security issue because a proxy in front of the http server could miss the fact that the path will be interpreted differently by it. Correct me if I'm wrong though.

The current workaround is to use RequestURI.

Screenshots

Screenshot 2019-09-06 at 19 56 02
john8329 commented 3 years ago

As of gin v1.7.1, this is still present, and a significant flaw in my opinion. I'm researching if a combination of the UseRawPath = true and UnescapePathValues = false solves this.

rcollette commented 1 year ago

Associated with PRISMA-2022-0394

github.com/gin-gonic/gin module from all versions is vulnerable to Path Traversal due to wildcard parameters incorrectly decoding URIs recursively. The wildcard request parameter should get the full path after the \\"/\\". If the path will contain some URL Encoded slashes (%2F), they get recursively decoded, which is incorrect and could lead to a Path Traversal issue.

Cookiery commented 1 year ago

@rcollette @john8329
You can use router.UseRawPath = true router.UnescapePathValues = false a example:

package main

import (
    "fmt"
    "net/http"

    "github.com/gin-gonic/gin"
)

func main() {
    r := gin.Default()
    r.GET("/*request", func(c *gin.Context) {
        request := c.Param("request")
        fmt.Println(request)
        c.JSON(http.StatusOK, gin.H{
            "message": "pong",
        })
    })

        // add this two line will fix your problem
    r.UseRawPath = true
    r.UnescapePathValues = false

    r.Run() // listen and serve on 0.0.0.0:8080 (for windows "localhost:8080")
}
joshriverscambia2019 commented 1 year ago

@Cookiery that looks like a solid workaround, but will not resolve the security warnings in enterprise scanning tools.

Is setting these as default a breaking change? Or do we need a basic fix to the url decoding?

Tak1za commented 1 year ago

Do we have a fix for this planned in any upcoming releases? Not all security teams in enterprises approve deployments with a dirty report even if a code fix is available.

joshriverscambia2019 commented 1 year ago

I'm tracking this as well. To be fair, when a vulnerability is tagged as PRISMA-xxx (this one is called PRISMA-2022-0393), it means that that security tool vendor has flagged it as an issue...but that nobody else has agreed. It would have a proper CVE if it was a real vulnerability.

I've been tracking this issue against my project, and if my compliance folks play hardball, then I'd have to decide if I'm removing gin as a dependency. I don't see how this is actually something the project maintainers should have to fix because our security tool is over-enthusiastic. I guess someone could maintain a patched fork of gin that contains the breaking(?) changes (either from the PR or as suggested by Cookiery).