diff --git a/go.mod b/go.mod index 8a7923b..5454f5d 100644 --- a/go.mod +++ b/go.mod @@ -9,7 +9,6 @@ require ( github.com/go-pkgz/repeater v1.1.3 github.com/go-pkgz/rest v1.17.0 github.com/golang-jwt/jwt v3.2.2+incompatible - github.com/microcosm-cc/bluemonday v1.0.26 github.com/nullrocks/identicon v0.0.0-20180626043057-7875f45b0022 github.com/stretchr/testify v1.8.4 go.etcd.io/bbolt v1.3.7 @@ -21,12 +20,10 @@ require ( require ( cloud.google.com/go/compute v1.23.1 // indirect cloud.google.com/go/compute/metadata v0.2.3 // indirect - github.com/aymerick/douceur v0.2.0 // indirect github.com/davecgh/go-spew v1.1.1 // indirect github.com/golang/protobuf v1.5.3 // indirect github.com/golang/snappy v0.0.4 // indirect github.com/google/uuid v1.1.1 // indirect - github.com/gorilla/css v1.0.0 // indirect github.com/klauspost/compress v1.17.1 // indirect github.com/montanaflynn/stats v0.7.1 // indirect github.com/pmezard/go-difflib v1.0.0 // indirect @@ -43,7 +40,6 @@ require ( github.com/xdg-go/stringprep v1.0.4 // indirect github.com/youmark/pkcs8 v0.0.0-20201027041543-1326539a0a0a // indirect golang.org/x/crypto v0.14.0 // indirect - golang.org/x/net v0.17.0 // indirect golang.org/x/sync v0.4.0 // indirect golang.org/x/sys v0.13.0 // indirect golang.org/x/text v0.13.0 // indirect diff --git a/go.sum b/go.sum index 4ca853b..e8bc980 100644 --- a/go.sum +++ b/go.sum @@ -7,8 +7,6 @@ github.com/ajg/form v1.5.1 h1:t9c7v8JUKu/XxOGBU0yjNpaMloxGEJhUkqFRq0ibGeU= github.com/ajg/form v1.5.1/go.mod h1:uL1WgH+h2mgNtvBq0339dVnzXdBETtL2LeUXaIv25UY= github.com/andybalholm/brotli v1.0.4 h1:V7DdXeJtZscaqfNuAdSRuRFzuiKlHSC/Zh3zl9qY3JY= github.com/andybalholm/brotli v1.0.4/go.mod h1:fO7iG3H7G2nSZ7m0zPUDn85XEX2GTukHGRSepvi9Eig= -github.com/aymerick/douceur v0.2.0 h1:Mv+mAeH1Q+n9Fr+oyamOlAkUNPWPlA8PPGR0QAaYuPk= -github.com/aymerick/douceur v0.2.0/go.mod h1:wlT5vV2O3h55X9m7iVYN0TBM0NH/MmbLnd30/FjWUq4= github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c= github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= @@ -61,8 +59,6 @@ github.com/google/uuid v1.1.1 h1:Gkbcsh/GbpXz7lPftLA3P6TYMwjCLYm83jiFQZF/3gY= github.com/google/uuid v1.1.1/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo= github.com/gopherjs/gopherjs v0.0.0-20181017120253-0766667cb4d1 h1:EGx4pi6eqNxGaHF6qqu48+N2wcFQ5qg5FXgOdqsJ5d8= github.com/gopherjs/gopherjs v0.0.0-20181017120253-0766667cb4d1/go.mod h1:wJfORRmW1u3UXTncJ5qlYoELFm8eSnnEO6hX4iZ3EWY= -github.com/gorilla/css v1.0.0 h1:BQqNyPTi50JCFMTw/b67hByjMVXZRwGha6wxVGkeihY= -github.com/gorilla/css v1.0.0/go.mod h1:Dn721qIggHpt4+EFCcTLTU/vk5ySda2ReITrtgBl60c= github.com/gorilla/websocket v1.4.2 h1:+/TMaTYc4QFitKJxsQ7Yye35DkWvkdLcvGKqM+x0Ufc= github.com/gorilla/websocket v1.4.2/go.mod h1:YR8l580nyteQvAITg2hZ9XVh4b55+EU/adAjf1fMHhE= github.com/hpcloud/tail v1.0.0/go.mod h1:ab1qPbhIpdTxEkNHXyeSf5vhxWSCs/tWer42PpOxQnU= @@ -82,8 +78,6 @@ github.com/kr/text v0.1.0 h1:45sCR5RtlFHMR4UwH9sdQ5TC8v0qDQCHnXt+kaKSTVE= github.com/kr/text v0.1.0/go.mod h1:4Jbv+DJW3UT/LiOwJeYQe1efqtUx/iVham/4vfdArNI= github.com/mattn/go-colorable v0.1.7/go.mod h1:u6P/XSegPjTcexA+o6vUJrdnUu04hMope9wVRipJSqc= github.com/mattn/go-isatty v0.0.12/go.mod h1:cbi8OIDigv2wuxKPP5vlRcQ1OAZbq2CE4Kysco4FUpU= -github.com/microcosm-cc/bluemonday v1.0.26 h1:xbqSvqzQMeEHCqMi64VAs4d8uy6Mequs3rQ0k/Khz58= -github.com/microcosm-cc/bluemonday v1.0.26/go.mod h1:JyzOCs9gkyQyjs+6h10UEVSe02CGwkhd72Xdqh78TWs= github.com/montanaflynn/stats v0.0.0-20171201202039-1bf9dbcd8cbe/go.mod h1:wL8QJuTMNUDYhXwkmfOly8iTdp5TEcJFWZD2D7SIkUc= github.com/montanaflynn/stats v0.7.1 h1:etflOAAHORrCC44V+aR6Ftzort912ZU+YLiSTuV8eaE= github.com/montanaflynn/stats v0.7.1/go.mod h1:etXPPgVO6n31NxCd9KQUMvCM+ve0ruNzt6R8Bnaayow= diff --git a/provider/custom_server.go b/provider/custom_server.go index ac49f02..64b90bc 100644 --- a/provider/custom_server.go +++ b/provider/custom_server.go @@ -2,6 +2,7 @@ package provider import ( "context" + "encoding/json" "fmt" "html/template" "net" @@ -12,7 +13,6 @@ import ( "time" goauth2 "github.com/go-oauth2/oauth2/v4/server" - "github.com/microcosm-cc/bluemonday" "golang.org/x/oauth2" "github.com/go-pkgz/auth/avatar" @@ -160,16 +160,19 @@ func (c *CustomServer) handleUserInfo(w http.ResponseWriter, r *http.Request) { } userID := ti.GetUserID() - p := bluemonday.UGCPolicy() - ava := p.Sanitize(fmt.Sprintf(c.URL+"/avatar?user=%s", userID)) - res := fmt.Sprintf(`{ - "id": "%s", - "name":"%s", - "picture":"%s" - }`, userID, userID, ava) + user := token.User{ + ID: userID, + Name: userID, + Picture: fmt.Sprintf(c.URL+"/avatar?user=%s", userID), + } + res, err := json.Marshal(user) + if err != nil { + w.WriteHeader(http.StatusInternalServerError) + return + } w.Header().Set("Content-Type", "application/json; charset=utf-8") - if _, err := w.Write([]byte(res)); err != nil { + if _, err := w.Write(res); err != nil { w.WriteHeader(http.StatusInternalServerError) return } diff --git a/provider/verify.go b/provider/verify.go index 23da826..8b0a03d 100644 --- a/provider/verify.go +++ b/provider/verify.go @@ -11,7 +11,6 @@ import ( "github.com/go-pkgz/rest" "github.com/golang-jwt/jwt" - "github.com/microcosm-cc/bluemonday" "github.com/go-pkgz/auth/avatar" "github.com/go-pkgz/auth/logger" @@ -134,9 +133,7 @@ func (e VerifyHandler) LoginHandler(w http.ResponseWriter, r *http.Request) { // GET /login?site=site&user=name&address=someone@example.com func (e VerifyHandler) sendConfirmation(w http.ResponseWriter, r *http.Request) { - user, address := r.URL.Query().Get("user"), r.URL.Query().Get("address") - user = e.sanitize(user) - address = e.sanitize(address) + user, address, site := r.URL.Query().Get("user"), r.URL.Query().Get("address"), r.URL.Query().Get("site") if user == "" || address == "" { rest.SendErrorJSON(w, r, e.L, http.StatusBadRequest, fmt.Errorf("wrong request"), "can't get user and address") @@ -150,7 +147,7 @@ func (e VerifyHandler) sendConfirmation(w http.ResponseWriter, r *http.Request) }, SessionOnly: r.URL.Query().Get("session") != "" && r.URL.Query().Get("session") != "0", StandardClaims: jwt.StandardClaims{ - Audience: e.sanitize(r.URL.Query().Get("site")), + Audience: site, ExpiresAt: time.Now().Add(30 * time.Minute).Unix(), NotBefore: time.Now().Add(-1 * time.Minute).Unix(), Issuer: e.Issuer, @@ -179,10 +176,10 @@ func (e VerifyHandler) sendConfirmation(w http.ResponseWriter, r *http.Request) Token string Site string }{ - User: user, - Address: address, + User: trim(user), + Address: trim(address), Token: tkn, - Site: r.URL.Query().Get("site"), + Site: site, } buf := bytes.Buffer{} if err = emailTmpl.Execute(&buf, tmplData); err != nil { @@ -212,14 +209,8 @@ Confirmation for {{.User}} {{.Address}}, site {{.Site}} Token: {{.Token}} ` -func (e VerifyHandler) sanitize(inp string) string { - p := bluemonday.UGCPolicy() - res := p.Sanitize(inp) - res = template.HTMLEscapeString(res) - res = strings.ReplaceAll(res, "&", "&") - res = strings.ReplaceAll(res, """, "\"") - res = strings.ReplaceAll(res, "'", "'") - res = strings.ReplaceAll(res, "\n", "") +func trim(inp string) string { + res := strings.ReplaceAll(inp, "\n", "") res = strings.TrimSpace(res) if len(res) > 128 { return res[:128] diff --git a/provider/verify_test.go b/provider/verify_test.go index 4779672..291c892 100644 --- a/provider/verify_test.go +++ b/provider/verify_test.go @@ -4,6 +4,7 @@ import ( "fmt" "net/http" "net/http/httptest" + "net/url" "strings" "testing" "time" @@ -59,7 +60,7 @@ func TestVerifyHandler_LoginSendConfirm(t *testing.T) { assert.Equal(t, "test", e.Name()) } -func TestVerifyHandler_LoginSendConfirmRejected(t *testing.T) { +func TestVerifyHandler_LoginSendConfirmEscapesBadInput(t *testing.T) { emailer := mockSender{} e := VerifyHandler{ @@ -77,20 +78,22 @@ func TestVerifyHandler_LoginSendConfirmRejected(t *testing.T) { handler := http.HandlerFunc(e.LoginHandler) rr := httptest.NewRecorder() - badUser := "%3C%21DOCTYPE%20html%3E%20%0A%3Chtml%3E%20%0A%3Chead%3E%0A%3Cmeta%20name%3D%22viewport%22%20content%3D%22width%3Ddevice-width%2C%20initial-scale%3D1%22%3E%0A%3Ctitle%3E%20Login%20Page%20%3C%2Ftitle%3E%0A%3Cstyle%3E%20%0ABody%20%7B%0A%20%20font-family%3A%20Calibri%2C%20Helvetica%2C%20sans-serif%3B%0A%20%20background-color%3A%20pink%3B%0A%7D%0Abutton%20%7B%20%0A%20%20%20%20%20%20%20background-color%3A%20%234CAF50%3B%20%0A%20%20%20%20%20%20%20width%3A%20100%25%3B%0A%20%20%20%20%20%20%20%20color%3A%20orange%3B%20%0A%20%20%20%20%20%20%20%20padding%3A%2015px%3B%20%0A%20%20%20%20%20%20%20%20margin%3A%2010px%200px%3B%20%0A%20%20%20%20%20%20%20%20border%3A%20none%3B%20%0A%20%20%20%20%20%20%20%20cursor%3A%20pointer%3B%20%0A%20%20%20%20%20%20%20%20%20%7D%20%0A%20form%20%7B%20%0A%20%20%20%20%20%20%20%20border%3A%203px%20solid%20%23f1f1f1%3B%20%0A%20%20%20%20%7D%20%0A%20input%5Btype%3Dtext%5D%2C%20input%5Btype%3Dpassword%5D%20%7B%20%0A%20%20%20%20%20%20%20%20width%3A%20100%25%3B%20%0A%20%20%20%20%20%20%20%20margin%3A%208px%200%3B%0A%20%20%20%20%20%20%20%20padding%3A%2012px%2020px%3B%20%0A%20%20%20%20%20%20%20%20display%3A%20inline-block%3B%20%0A%20%20%20%20%20%20%20%20border%3A%202px%20solid%20green%3B%20%0A%20%20%20%20%20%20%20%20box-sizing%3A%20border-box%3B%20%0A%20%20%20%20%7D%0A%20button%3Ahover%20%7B%20%0A%20%20%20%20%20%20%20%20opacity%3A%200.7%3B%20%0A%20%20%20%20%7D%20%0A%20%20.cancelbtn%20%7B%20%0A%20%20%20%20%20%20%20%20width%3A%20auto%3B%20%0A%20%20%20%20%20%20%20%20padding%3A%2010px%2018px%3B%0A%20%20%20%20%20%20%20%20margin%3A%2010px%205px%3B%0A%20%20%20%20%7D%20%0A%20%20%20%20%20%20%0A%20%20%20%0A%20.container%20%7B%20%0A%20%20%20%20%20%20%20%20padding%3A%2025px%3B%20%0A%20%20%20%20%20%20%20%20background-color%3A%20lightblue%3B%0A%20%20%20%20%7D%20%0A%3C%2Fstyle%3E%20%0A%3C%2Fhead%3E%20%20%0A%3Cbody%3E%20%20%0A%20%20%20%20%3Ccenter%3E%20%3Ch1%3E%20Student%20Login%20Form%20%3C%2Fh1%3E%20%3C%2Fcenter%3E%20%0A%20%20%20%20%3Cform%3E%0A%20%20%20%20%20%20%20%20%3Cdiv%20class%3D%22container%22%3E%20%0A%20%20%20%20%20%20%20%20%20%20%20%20%3Clabel%3EUsername%20%3A%20%3C%2Flabel%3E%20%0A%20%20%20%20%20%20%20%20%20%20%20%20%3Cinput%20type%3D%22text%22%20placeholder%3D%22Enter%20Username%22%20name%3D%22username%22%20required%3E%0A%20%20%20%20%20%20%20%20%20%20%20%20%3Clabel%3EPassword%20%3A%20%3C%2Flabel%3E%20%0A%20%20%20%20%20%20%20%20%20%20%20%20%3Cinput%20type%3D%22password%22%20placeholder%3D%22Enter%20Password%22%20name%3D%22password%22%20required%3E%0A%20%20%20%20%20%20%20%20%20%20%20%20%3Cbutton%20type%3D%22submit%22%3ELogin%3C%2Fbutton%3E%20%0A%20%20%20%20%20%20%20%20%20%20%20%20%3Cinput%20type%3D%22checkbox%22%20checked%3D%22checked%22%3E%20Remember%20me%20%0A%20%20%20%20%20%20%20%20%20%20%20%20%3Cbutton%20type%3D%22button%22%20class%3D%22cancelbtn%22%3E%20Cancel%3C%2Fbutton%3E%20%0A%20%20%20%20%20%20%20%20%20%20%20%20Forgot%20%3Ca%20href%3D%22%23%22%3E%20password%3F%20%3C%2Fa%3E%20%0A%20%20%20%20%20%20%20%20%3C%2Fdiv%3E%20%0A%20%20%20%20%3C%2Fform%3E%20%20%20%0A%3C%2Fbody%3E%20%20%20%0A%3C%2Fhtml%3E%0A%0A%20%0A" - req, err := http.NewRequest("GET", "/login?address=blah@user.com&user="+badUser+"&site=remark42", http.NoBody) + badData := "<escaped>" + req, err := http.NewRequest("GET", "/login?address=blah@user.com&user="+url.QueryEscape(badData)+"&site="+url.QueryEscape(badData), http.NoBody) require.NoError(t, err) handler.ServeHTTP(rr, req) assert.Equal(t, 200, rr.Code) assert.Equal(t, "blah@user.com", emailer.to) - assert.Contains(t, emailer.text, "Password : blah@user.com remark42 token:") + expectedEscaped := "<html><script>nasty stuff</script>&lt;escaped&gt;</html>" + assert.Contains(t, emailer.text, expectedEscaped+" blah@user.com "+expectedEscaped+" token:") tknStr := strings.Split(emailer.text, " token:")[1] tkn, err := e.TokenService.Parse(tknStr) assert.NoError(t, err) t.Logf("%s %+v", tknStr, tkn) - assert.Equal(t, "<h1> Student Login Form </h1> <div> Username : Password : ::blah@user.com", tkn.Handshake.ID) - assert.Equal(t, "remark42", tkn.Audience) + // not escaped in these fields as they are not rendered as HTML + assert.Equal(t, badData+"::blah@user.com", tkn.Handshake.ID) + assert.Equal(t, badData, tkn.Audience) assert.True(t, tkn.ExpiresAt > tkn.NotBefore) assert.Equal(t, "test", e.Name())