Skip to content

Commit ef8d461

Browse files
committed
fix xsrf issues on link update and delete
Two fixes, both of which would be sufficient on their own, but are both still worth doing: - redirect /.detail/ URLs to always use canonical link names. For example, for a go/foo link, a request to /.detail/F-O-O will redirect to /.detail/foo. - use the canonical link short name for xsrf token generation and validation, rather than the user-provided short name. Fixes #128 Signed-off-by: Will Norris <[email protected]>
1 parent 55a3c6e commit ef8d461

File tree

1 file changed

+16
-11
lines changed

1 file changed

+16
-11
lines changed

golink.go

Lines changed: 16 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -563,6 +563,11 @@ func serveDetail(w http.ResponseWriter, r *http.Request) {
563563
http.NotFound(w, r)
564564
return
565565
}
566+
if short != link.Short {
567+
// redirect to canonical short name
568+
http.Redirect(w, r, "/.detail/"+link.Short, http.StatusFound)
569+
return
570+
}
566571
if err != nil {
567572
log.Printf("serving detail %q: %v", short, err)
568573
http.Error(w, err.Error(), http.StatusInternalServerError)
@@ -591,7 +596,7 @@ func serveDetail(w http.ResponseWriter, r *http.Request) {
591596
data := detailData{
592597
Link: link,
593598
Editable: canEdit,
594-
XSRF: xsrftoken.Generate(xsrfKey, cu.login, short),
599+
XSRF: xsrftoken.Generate(xsrfKey, cu.login, link.Short),
595600
}
596601
if canEdit && !ownerExists {
597602
data.Link.Owner = cu.login
@@ -777,7 +782,7 @@ func serveDelete(w http.ResponseWriter, r *http.Request) {
777782
// want to enable deletion via CLI and to honor allowUnknownUsers for
778783
// deletion, we could change the below to a call to isRequestAuthorized. For
779784
// now, always require the XSRF token, thus maintaining the status quo.
780-
if !xsrftoken.Valid(r.PostFormValue("xsrf"), xsrfKey, cu.login, short) {
785+
if !xsrftoken.Valid(r.PostFormValue("xsrf"), xsrfKey, cu.login, link.Short) {
781786
http.Error(w, "invalid XSRF token", http.StatusBadRequest)
782787
return
783788
}
@@ -829,7 +834,14 @@ func serveSave(w http.ResponseWriter, r *http.Request) {
829834
return
830835
}
831836

832-
if !isRequestAuthorized(r, cu, short) {
837+
// short name to use for XSRF token.
838+
// For new link creation, the special newShortName value is used.
839+
tokenShortName := newShortName
840+
if link != nil {
841+
tokenShortName = link.Short
842+
}
843+
844+
if !isRequestAuthorized(r, cu, tokenShortName) {
833845
http.Error(w, "invalid XSRF token", http.StatusBadRequest)
834846
return
835847
}
@@ -978,12 +990,5 @@ func isRequestAuthorized(r *http.Request, u user, short string) bool {
978990
return true
979991
}
980992

981-
// If the request is to create a new link, test the XSRF token against
982-
// newShortName instead of short.
983-
tokenShortName := short
984-
_, err := db.Load(short)
985-
if r.URL.Path == "/" && r.Method == http.MethodPost && errors.Is(err, fs.ErrNotExist) {
986-
tokenShortName = newShortName
987-
}
988-
return xsrftoken.Valid(r.PostFormValue("xsrf"), xsrfKey, u.login, tokenShortName)
993+
return xsrftoken.Valid(r.PostFormValue("xsrf"), xsrfKey, u.login, short)
989994
}

0 commit comments

Comments
 (0)