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

net/url: Parse accepts invalid userinfo strings #23392

Closed
bradfitz opened this issue Jan 9, 2018 · 9 comments
Closed

net/url: Parse accepts invalid userinfo strings #23392

bradfitz opened this issue Jan 9, 2018 · 9 comments
Labels
CherryPickApproved Used during the release process for point releases FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done. Security
Milestone

Comments

@bradfitz
Copy link
Contributor

bradfitz commented Jan 9, 2018

@adamdecaf reported that net/url.Parse accepts URLs with userinfo components containing just about anything (newlines and random non-ASCII Unicode).

This could be a security problem if people use the resulting URL.User.Username & Password without further validation.

@bradfitz bradfitz added Security NeedsFix The path to resolution is known, but the work has not been done. labels Jan 9, 2018
@bradfitz bradfitz added this to the Go1.10 milestone Jan 9, 2018
@bradfitz bradfitz self-assigned this Jan 9, 2018
@gopherbot
Copy link

Change https://golang.org/cl/87038 mentions this issue: net/url: reject invalid userinfo values when parsing URLs

@bradfitz bradfitz modified the milestones: Go1.10, Go1.9.3 Jan 10, 2018
@bradfitz bradfitz reopened this Jan 10, 2018
@andybons
Copy link
Member

andybons commented Jan 18, 2018

CL 88535 OK for Go 1.9.3.

@andybons andybons added the CherryPickApproved Used during the release process for point releases label Jan 18, 2018
@gopherbot
Copy link

Change https://golang.org/cl/88535 mentions this issue: [release-branch.go1.9] net/url: reject invalid userinfo values when parsing URLs

gopherbot pushed a commit that referenced this issue Jan 22, 2018
…arsing URLs

Fixes #23392

Change-Id: I5822b082b14d886b9c3b5ad7beebb2c01a77851b
Reviewed-on: https://go-review.googlesource.com/87038
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Reviewed-on: https://go-review.googlesource.com/88535
Run-TryBot: Andrew Bonventre <andybons@golang.org>
@andybons
Copy link
Member

go1.9.3 has been packaged and includes:

  • CL 88535 [release-branch.go1.9] net/url: reject invalid userinfo values when parsing URLs

The release is posted at golang.org/dl.

— golang.org/x/build/cmd/releasebot, Jan 22 21:02:59 UTC

@ernsheong
Copy link

ernsheong commented Jan 27, 2018

My application stopped talking to the database. Turns out database password contained a [ which isn't whitelisted 😭 Bad idea to have symbols in DB password I guess.

@adamdecaf
Copy link
Contributor

Oh.. How were you parsing/handling the uri?

@ernsheong
Copy link

I was using https://github.com/jackc/pgx, which was using net/url to do it

@adamdecaf
Copy link
Contributor

adamdecaf commented Jan 28, 2018

Percent encoding passes fine for me. From https://stackoverflow.com/questions/23353623/how-to-handle-special-characters-in-the-password-of-a-postgresql-url-connection

package main

import (
	"fmt"
	"net/url"
)

func main() {
	// raw := "postgresql://daniel:p$ass@localhost/test"
	raw := "postgresql://daniel:p%24ass@localhost"

	fmt.Printf("RAW: %q\n", raw)
	u, err := url.Parse(raw)
	if err != nil {
		fmt.Printf("ERROR: %v\n", err)
	}
	fmt.Println(u.String())
}
$ go run /tmp/jdbc.go 
RAW: "postgresql://daniel:p%24ass@localhost"
postgresql://daniel:p$ass@localhost

Edit: I tried with ] also.

$ go run /tmp/jdbc.go 
RAW: "postgresql://daniel:p%5Bass@localhost"
postgresql://daniel:p%5Bass@localhost

@AlphaWong
Copy link

I would like to know did all those changes pass the test in OSX as I can get an & operator in arch Linux but my workmate get a \u0026 in his macbook.

After I use the docker image 1.8-apline
This issue got.

I will update the screen shot later

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
CherryPickApproved Used during the release process for point releases FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done. Security
Projects
None yet
Development

No branches or pull requests

6 participants