CVE-2019-8400: Improper Neutralization of Input During Web Page Generation ('Cross-site Scripting') (CWE-79)
value: ORY Hydra before v1.0.0-rc.3+oryOS.9 has Reflected XSS via the oauth2/fallbacks/error error_hint parameter.
CWE description: The product does not neutralize or incorrectly neutralizes user-controllable input before it is placed in output that is used as a web page that is served to other users.
CVSS v3 base score: 6.1
Published: 2019-02-17T06:29Z
Repository: https://github.com/ory/hydra
Language: Other
Files changed: go.mod, go.sum, oauth2/handler_fallback_endpoints.go
Commit date: 2018-12-06 10:18:48 +0100
Commit message: oauth2: Use html templates in fallback endpoints (#1202)

Signed-off-by: aeneasr <aeneas@ory.sh>
Diff: diff --git a/go.mod b/go.mod
index b901cb48..e958aa8c 100644
--- a/go.mod
+++ b/go.mod
@@ -40,11 +40,11 @@ require (
 	github.com/uber-go/atomic v1.3.2 // indirect
 	github.com/uber/jaeger-client-go v2.15.0+incompatible
 	github.com/urfave/negroni v1.0.0
 	github.com/ziutek/mymysql v1.5.4 // indirect
 	go.uber.org/atomic v1.3.2 // indirect
-	golang.org/x/crypto v0.0.0-20181001203147-e3636079e1a4
+	golang.org/x/crypto v0.0.0-20181203042331-505ab145d0a9
 	golang.org/x/net v0.0.0-20181029044818-c44066c5c816 // indirect
 	golang.org/x/oauth2 v0.0.0-20181003184128-c57b0facaced
 	gopkg.in/check.v1 v1.0.0-20180628173108-788fd7840127 // indirect
 	gopkg.in/resty.v1 v1.9.1
 	gopkg.in/square/go-jose.v2 v2.1.9
diff --git a/go.sum b/go.sum
index 9734805b..fcf23067 100644
--- a/go.sum
+++ b/go.sum
@@ -264,10 +264,12 @@ go.uber.org/atomic v1.3.2/go.mod h1:gD2HeocX3+yG+ygLZcrzQJaqmWj9AIm7n08wl/qW/PE=
 golang.org/x/crypto v0.0.0-20180830192347-182538f80094 h1:rVTAlhYa4+lCfNxmAIEOGQRoD23UqP72M3+rSWVGDTg=
 golang.org/x/crypto v0.0.0-20180830192347-182538f80094/go.mod h1:6SG95UA2DQfeDnfUPMdvaQW0Q7yPrPDi9nlGo2tz2b4=
 golang.org/x/crypto v0.0.0-20180904163835-0709b304e793/go.mod h1:6SG95UA2DQfeDnfUPMdvaQW0Q7yPrPDi9nlGo2tz2b4=
 golang.org/x/crypto v0.0.0-20181001203147-e3636079e1a4 h1:Vk3wNqEZwyGyei9yq5ekj7frek2u7HUfffJ1/opblzc=
 golang.org/x/crypto v0.0.0-20181001203147-e3636079e1a4/go.mod h1:6SG95UA2DQfeDnfUPMdvaQW0Q7yPrPDi9nlGo2tz2b4=
+golang.org/x/crypto v0.0.0-20181203042331-505ab145d0a9 h1:mKdxBk7AujPs8kU4m80U72y/zjbZ3UcXC7dClwKbUI0=
+golang.org/x/crypto v0.0.0-20181203042331-505ab145d0a9/go.mod h1:6SG95UA2DQfeDnfUPMdvaQW0Q7yPrPDi9nlGo2tz2b4=
 golang.org/x/net v0.0.0-20180530234432-1e491301e022/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4=
 golang.org/x/net v0.0.0-20180611182652-db08ff08e862 h1:JZi6BqOZ+iSgmLWe6llhGrNnEnK+YB/MRkStwnEfbqM=
 golang.org/x/net v0.0.0-20180611182652-db08ff08e862/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4=
 golang.org/x/net v0.0.0-20180724234803-3673e40ba225/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4=
 golang.org/x/net v0.0.0-20180906233101-161cd47e91fd/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4=
diff --git a/main_test.go.bak b/main_test.go.bak
deleted file mode 100644
index 7e395dd7..00000000
--- a/main_test.go.bak
+++ /dev/null
@@ -1,78 +0,0 @@
-package main_test
-
-import (
-	"fmt"
-	"github.com/jmoiron/sqlx"
-	"github.com/ory/fosite"
-	"github.com/ory/hydra/client"
-	"github.com/ory/hydra/consent"
-	"github.com/ory/hydra/jwk"
-	"github.com/ory/hydra/oauth2"
-	"github.com/ory/x/resilience"
-	"github.com/sirupsen/logrus"
-	"github.com/stretchr/testify/require"
-	"os"
-	"testing"
-	"time"
-)
-
-func connect(t *testing.T, driver, dsn string) (db *sqlx.DB) {
-	require.NoError(t, resilience.Retry(
-		logrus.New(),
-		time.Second * 5,
-		time.Minute * 5,
-		func() (err error) {
-			db, err = sqlx.Open(driver, dsn)
-			if err != nil {
-				return err
-			}
-
-			return db.Ping()
-		},
-	))
-	return db
-}
-
-func TestCreateSchemasAtRoot(t *testing.T) {
-	if testing.Short() {
-		t.Skip("Skipping database tests in short mode.")
-		return
-	}
-
-	drivers := map[string]string{
-		"TEST_DATABASE_POSTGRESQL": "postgres",
-		"TEST_DATABASE_MYSQL":      "mysql",
-	}
-	urls := map[string]string{
-		"TEST_DATABASE_POSTGRESQL": os.Getenv("TEST_DATABASE_POSTGRESQL"),
-		"TEST_DATABASE_MYSQL":      os.Getenv("TEST_DATABASE_MYSQL"),
-	}
-
-	for e, u := range urls {
-		if len(u) == 0 {
-			t.Fatalf("Environment variable %s is empty but it should point to a database. If you are running this test locally, make sure to run `make test` instead of `go test ./...`", e)
-		}
-
-		driver := drivers[e]
-		t.Run(fmt.Sprintf("driver=%s", driver), func(t *testing.T) {
-			t.Logf("Connecting to database %s: %s", driver, u)
-
-			db := connect(t, driver, u)
-			cm := &client.SQLManager{DB: db, Hasher: &fosite.BCrypt{}}
-			_, err := cm.CreateSchemas()
-			require.NoError(t, err)
-
-			jm := &jwk.SQLManager{DB: db, Cipher: &jwk.AEAD{Key: []byte("11111111111111111111111111111111")}}
-			_, err = jm.CreateSchemas()
-			require.NoError(t, err)
-
-			om := &oauth2.FositeSQLStore{Manager: cm, DB: db, L: logrus.New()}
-			_, err = om.CreateSchemas()
-			require.NoError(t, err)
-
-			crm := consent.NewSQLManager(db, nil, om)
-			_, err = crm.CreateSchemas()
-			require.NoError(t, err)
-		})
-	}
-}
diff --git a/oauth2/handler_fallback_endpoints.go b/oauth2/handler_fallback_endpoints.go
index 7b2103a5..55596bab 100644
--- a/oauth2/handler_fallback_endpoints.go
+++ b/oauth2/handler_fallback_endpoints.go
@@ -19,21 +19,21 @@
  */
 
 package oauth2
 
 import (
-	"fmt"
+	"html/template"
 	"net/http"
 
 	"github.com/julienschmidt/httprouter"
 )
 
 func (h *Handler) DefaultConsentHandler(w http.ResponseWriter, r *http.Request, _ httprouter.Params) {
 	h.L.Warnln("It looks like no consent/login URL was set. All OAuth2 flows except client credentials will fail.")
 	h.L.Warnln("A client requested the default login & consent URL, environment variable OAUTH2_CONSENT_URL or OAUTH2_LOGIN_URL or both are probably not set.")
 
-	w.Write([]byte(`
+	t, err := template.New("consent").Parse(`
 <html>
 <head>
 	<title>Misconfigured consent/login URL</title>
 </head>
 <body>
@@ -45,45 +45,72 @@ func (h *Handler) DefaultConsentHandler(w http.ResponseWriter, r *http.Request,
 	If you are an administrator, please read <a href="https://www.ory.sh/docs">
 	the guide</a> to understand what you need to do. If you are a user, please contact the administrator.
 </p>
 </body>
 </html>
-`))
+`)
+	if err != nil {
+		h.H.WriteError(w, r, err)
+		return
+	}
+
+	if err := t.Execute(w, nil); err != nil {
+		h.H.WriteError(w, r, err)
+		return
+	}
 }
 
 func (h *Handler) DefaultErrorHandler(w http.ResponseWriter, r *http.Request, _ httprouter.Params) {
 	h.L.Warnln("A client requested the default error URL, environment variable OAUTH2_ERROR_URL is probably not set.")
 
-	fmt.Fprintf(w, `
+	t, err := template.New("consent").Parse(`
 <html>
 <head>
 	<title>An OAuth 2.0 Error Occurred</title>
 </head>
 <body>
 <h1>
 	The OAuth2 request resulted in an error.
 </h1>
 <ul>
-	<li>Error: %s</li>
-	<li>Description: %s</li>
-	<li>Hint: %s</li>
-	<li>Debug: %s</li>
+	<li>Error: {{ .Name }}</li>
+	<li>Description: {{ .Description }}</li>
+	<li>Hint: {{ .Hint }}</li>
+	<li>Debug: {{ .Debug }}</li>
 </ul>
 <p>
 	You are seeing this default error page because the administrator has not set a dedicated error URL (environment variable <code>OAUTH2_ERROR_URL</code> is not set). 
 	If you are an administrator, please read <a href="https://www.ory.sh/docs">the guide</a> to understand what you
 	need to do. If you are a user, please contact the administrator.
 </p>
 </body>
 </html>
-`, r.URL.Query().Get("error"), r.URL.Query().Get("error_description"), r.URL.Query().Get("error_hint"), r.URL.Query().Get("error_debug"))
+`)
+	if err != nil {
+		h.H.WriteError(w, r, err)
+		return
+	}
+
+	if err := t.Execute(w, struct {
+		Name        string
+		Description string
+		Hint        string
+		Debug       string
+	}{
+		Name:        r.URL.Query().Get("error"),
+		Description: r.URL.Query().Get("error_description"),
+		Hint:        r.URL.Query().Get("error_hint"),
+		Debug:       r.URL.Query().Get("error_debug"),
+	}); err != nil {
+		h.H.WriteError(w, r, err)
+		return
+	}
 }
 
 func (h *Handler) DefaultLogoutHandler(w http.ResponseWriter, r *http.Request, _ httprouter.Params) {
 	h.L.Warnln("A client requested the default logout URL, environment variable OAUTH2_LOGOUT_REDIRECT_URL is probably not set.")
-
-	fmt.Fprintf(w, `
+	t, err := template.New("consent").Parse(`
 <html>
 <head>
 	<title>You logged out successfully</title>
 </head>
 <body>
@@ -96,6 +123,15 @@ func (h *Handler) DefaultLogoutHandler(w http.ResponseWriter, r *http.Request, _
 	need to do. If you are a user, please contact the administrator.
 </p>
 </body>
 </html>
 `)
+	if err != nil {
+		h.H.WriteError(w, r, err)
+		return
+	}
+
+	if err := t.Execute(w, nil); err != nil {
+		h.H.WriteError(w, r, err)
+		return
+	}
 }

Vulnerable code: 	golang.org/x/crypto v0.0.0-20181001203147-e3636079e1a4
package main_test

import (
	"fmt"
	"github.com/jmoiron/sqlx"
	"github.com/ory/fosite"
	"github.com/ory/hydra/client"
	"github.com/ory/hydra/consent"
	"github.com/ory/hydra/jwk"
	"github.com/ory/hydra/oauth2"
	"github.com/ory/x/resilience"
	"github.com/sirupsen/logrus"
	"github.com/stretchr/testify/require"
	"os"
	"testing"
	"time"
)

func connect(t *testing.T, driver, dsn string) (db *sqlx.DB) {
	require.NoError(t, resilience.Retry(
		logrus.New(),
		time.Second * 5,
		time.Minute * 5,
		func() (err error) {
			db, err = sqlx.Open(driver, dsn)
			if err != nil {
				return err
			}

			return db.Ping()
		},
	))
	return db
}

func TestCreateSchemasAtRoot(t *testing.T) {
	if testing.Short() {
		t.Skip("Skipping database tests in short mode.")
		return
	}

	drivers := map[string]string{
		"TEST_DATABASE_POSTGRESQL": "postgres",
		"TEST_DATABASE_MYSQL":      "mysql",
	}
	urls := map[string]string{
		"TEST_DATABASE_POSTGRESQL": os.Getenv("TEST_DATABASE_POSTGRESQL"),
		"TEST_DATABASE_MYSQL":      os.Getenv("TEST_DATABASE_MYSQL"),
	}

	for e, u := range urls {
		if len(u) == 0 {
			t.Fatalf("Environment variable %s is empty but it should point to a database. If you are running this test locally, make sure to run `make test` instead of `go test ./...`", e)
		}

		driver := drivers[e]
		t.Run(fmt.Sprintf("driver=%s", driver), func(t *testing.T) {
			t.Logf("Connecting to database %s: %s", driver, u)

			db := connect(t, driver, u)
			cm := &client.SQLManager{DB: db, Hasher: &fosite.BCrypt{}}
			_, err := cm.CreateSchemas()
			require.NoError(t, err)

			jm := &jwk.SQLManager{DB: db, Cipher: &jwk.AEAD{Key: []byte("11111111111111111111111111111111")}}
			_, err = jm.CreateSchemas()
			require.NoError(t, err)

			om := &oauth2.FositeSQLStore{Manager: cm, DB: db, L: logrus.New()}
			_, err = om.CreateSchemas()
			require.NoError(t, err)

			crm := consent.NewSQLManager(db, nil, om)
			_, err = crm.CreateSchemas()
			require.NoError(t, err)
		})
	}
}
	"fmt"
	w.Write([]byte(`
`))
	fmt.Fprintf(w, `
	<li>Error: %s</li>
	<li>Description: %s</li>
	<li>Hint: %s</li>
	<li>Debug: %s</li>
`, r.URL.Query().Get("error"), r.URL.Query().Get("error_description"), r.URL.Query().Get("error_hint"), r.URL.Query().Get("error_debug"))

	fmt.Fprintf(w, `
Fixed code: 	golang.org/x/crypto v0.0.0-20181203042331-505ab145d0a9
golang.org/x/crypto v0.0.0-20181203042331-505ab145d0a9 h1:mKdxBk7AujPs8kU4m80U72y/zjbZ3UcXC7dClwKbUI0=
golang.org/x/crypto v0.0.0-20181203042331-505ab145d0a9/go.mod h1:6SG95UA2DQfeDnfUPMdvaQW0Q7yPrPDi9nlGo2tz2b4=
	"html/template"
	t, err := template.New("consent").Parse(`
`)
	if err != nil {
		h.H.WriteError(w, r, err)
		return
	}

	if err := t.Execute(w, nil); err != nil {
		h.H.WriteError(w, r, err)
		return
	}
	t, err := template.New("consent").Parse(`
	<li>Error: {{ .Name }}</li>
	<li>Description: {{ .Description }}</li>
	<li>Hint: {{ .Hint }}</li>
	<li>Debug: {{ .Debug }}</li>
`)
	if err != nil {
		h.H.WriteError(w, r, err)
		return
	}

	if err := t.Execute(w, struct {
		Name        string
		Description string
		Hint        string
		Debug       string
	}{
		Name:        r.URL.Query().Get("error"),
		Description: r.URL.Query().Get("error_description"),
		Hint:        r.URL.Query().Get("error_hint"),
		Debug:       r.URL.Query().Get("error_debug"),
	}); err != nil {
		h.H.WriteError(w, r, err)
		return
	}
	t, err := template.New("consent").Parse(`
	if err != nil {
		h.H.WriteError(w, r, err)
		return
	}

	if err := t.Execute(w, nil); err != nil {
		h.H.WriteError(w, r, err)
		return
	}