From ad7439283cf203c901c31dbe764b9a3603fd1bd6 Mon Sep 17 00:00:00 2001 From: akuma06 Date: Tue, 30 May 2017 00:18:43 +0200 Subject: [PATCH 1/2] CSRF Exclusion As per suggestion of @yiiTT, CSRF is limited on users login, registration, profile edit, comments post, torrent edit. Uploads are not yet CSRF protected because api upload can't be used for that --- main.go | 9 ++--- router/router.go | 90 ++++++++++++++++++++++++++++++++---------------- 2 files changed, 62 insertions(+), 37 deletions(-) diff --git a/main.go b/main.go index b9b79b04..d86ade2e 100644 --- a/main.go +++ b/main.go @@ -21,7 +21,6 @@ import ( "github.com/NyaaPantsu/nyaa/util/publicSettings" "github.com/NyaaPantsu/nyaa/util/search" "github.com/NyaaPantsu/nyaa/util/signals" - "github.com/gorilla/csrf" ) // RunServer runs webapp mainloop @@ -31,12 +30,8 @@ func RunServer(conf *config.Config) { // TODO Use config from cli os.Mkdir(router.GPGPublicKeyPath, 700) - // Please make EnableSecureCSRF to false when testing locally - if config.EnableSecureCSRF { - http.Handle("/", csrf.Protect(config.CSRFTokenHashKey)(router.Router)) - } else { - http.Handle("/", csrf.Protect(config.CSRFTokenHashKey, csrf.Secure(false))(router.Router)) - } + http.Handle("/", router.Router) + // Set up server, srv := &http.Server{ WriteTimeout: 30 * time.Second, diff --git a/router/router.go b/router/router.go index f2301665..c6125636 100755 --- a/router/router.go +++ b/router/router.go @@ -3,7 +3,9 @@ package router import ( "net/http" + "github.com/NyaaPantsu/nyaa/config" "github.com/NyaaPantsu/nyaa/service/captcha" + "github.com/gorilla/csrf" "github.com/gorilla/handlers" "github.com/gorilla/mux" ) @@ -42,48 +44,74 @@ func init() { http.Handle("/img/", http.StripPrefix("/img/", imgHandler)) http.Handle("/dbdumps/", http.StripPrefix("/dbdumps/", wrapHandler(gzipDumpsHandler))) http.Handle("/gpg/", http.StripPrefix("/gpg/", wrapHandler(gzipGpgKeyHandler))) + + // We don't need CSRF here Router.Handle("/", gzipHomeHandler).Name("home") Router.Handle("/page/{page:[0-9]+}", wrapHandler(gzipHomeHandler)).Name("home_page") Router.HandleFunc("/search", SearchHandler).Name("search") Router.HandleFunc("/search/{page}", SearchHandler).Name("search_page") - Router.Handle("/api", wrapHandler(gzipAPIHandler)).Methods("GET") - Router.Handle("/api/{page:[0-9]*}", wrapHandler(gzipAPIHandler)).Methods("GET") - Router.Handle("/api/view/{id}", wrapHandler(gzipAPIViewHandler)).Methods("GET") - Router.HandleFunc("/api/view/{id}", APIViewHeadHandler).Methods("HEAD") - Router.HandleFunc("/api/upload", APIUploadHandler).Methods("POST") - Router.HandleFunc("/api/search", APISearchHandler) - Router.HandleFunc("/api/search/{page}", APISearchHandler) - Router.HandleFunc("/api/update", APIUpdateHandler).Methods("PUT") + Router.HandleFunc("/verify/email/{token}", UserVerifyEmailHandler).Name("user_verify").Methods("GET") Router.HandleFunc("/faq", FaqHandler).Name("faq") Router.HandleFunc("/feed", RSSHandler).Name("feed") Router.HandleFunc("/feed/{page}", RSSHandler).Name("feed_page") - Router.Handle("/view/{id}", wrapHandler(gzipViewHandler)).Methods("GET").Name("view_torrent") - Router.HandleFunc("/view/{id}", ViewHeadHandler).Methods("HEAD") - Router.HandleFunc("/view/{id}", PostCommentHandler).Methods("POST").Name("post_comment") - Router.HandleFunc("/torrent/", TorrentEditUserPanel).Methods("GET").Name("user_torrent_edit") - Router.HandleFunc("/torrent/", TorrentPostEditUserPanel).Methods("POST").Name("user_torrent_edit") - Router.HandleFunc("/torrent/delete", TorrentDeleteUserPanel).Methods("GET").Name("user_torrent_delete") - Router.HandleFunc("/upload", UploadHandler).Name("upload") - Router.HandleFunc("/user/register", UserRegisterFormHandler).Name("user_register").Methods("GET") - Router.HandleFunc("/user/login", UserLoginFormHandler).Name("user_login").Methods("GET") - Router.HandleFunc("/verify/email/{token}", UserVerifyEmailHandler).Name("user_verify").Methods("GET") - Router.HandleFunc("/user/register", UserRegisterPostHandler).Name("user_register").Methods("POST") - Router.HandleFunc("/user/login", UserLoginPostHandler).Name("user_login").Methods("POST") - Router.HandleFunc("/user/logout", UserLogoutHandler).Name("user_logout") - Router.Handle("/user/{id}/{username}", wrapHandler(gzipUserProfileHandler)).Name("user_profile").Methods("GET") - Router.HandleFunc("/user/{id}/{username}/follow", UserFollowHandler).Name("user_follow").Methods("GET") - Router.Handle("/user/{id}/{username}/edit", wrapHandler(gzipUserDetailsHandler)).Name("user_profile_details").Methods("GET") - Router.Handle("/user/{id}/{username}/edit", wrapHandler(gzipUserProfileFormHandler)).Name("user_profile_edit").Methods("POST") - Router.Handle("/user/{id}/{username}/apireset", wrapHandler(gzipUserAPIKeyResetHandler)).Name("user_profile_apireset").Methods("GET") - Router.Handle("/user/notifications", wrapHandler(gzipUserNotificationsHandler)).Name("user_notifications") - Router.HandleFunc("/user/{id}/{username}/feed", RSSHandler).Name("feed_user") - Router.HandleFunc("/user/{id}/{username}/feed/{page}", RSSHandler).Name("feed_user_page") // !!! This line need to have the same download location as the one define in config.TorrentStorageLink !!! Router.Handle("/download/{hash}", wrapHandler(downloadTorrentHandler)).Name("torrent_download") + // For now, no CSRF protection here, as API is not usable for uploads + Router.HandleFunc("/upload", UploadHandler).Name("upload") + + torrentViewRoutes := Router.PathPrefix("/view").Subrouter() + torrentViewRoutes.Handle("/{id}", wrapHandler(gzipViewHandler)).Methods("GET").Name("view_torrent") + torrentViewRoutes.HandleFunc("/{id}", ViewHeadHandler).Methods("HEAD") + torrentViewRoutes.HandleFunc("/{id}", PostCommentHandler).Methods("POST").Name("post_comment") + + torrentRoutes := Router.PathPrefix("/torrent").Subrouter() + torrentRoutes.HandleFunc("/", TorrentEditUserPanel).Methods("GET").Name("user_torrent_edit") + torrentRoutes.HandleFunc("/", TorrentPostEditUserPanel).Methods("POST").Name("user_torrent_edit") + torrentRoutes.HandleFunc("/delete", TorrentDeleteUserPanel).Methods("GET").Name("user_torrent_delete") + + userRoutes := Router.PathPrefix("/user").Subrouter() + userRoutes.HandleFunc("/register", UserRegisterFormHandler).Name("user_register").Methods("GET") + userRoutes.HandleFunc("/login", UserLoginFormHandler).Name("user_login").Methods("GET") + userRoutes.HandleFunc("/register", UserRegisterPostHandler).Name("user_register").Methods("POST") + userRoutes.HandleFunc("/login", UserLoginPostHandler).Name("user_login").Methods("POST") + userRoutes.HandleFunc("/logout", UserLogoutHandler).Name("user_logout") + userRoutes.Handle("/{id}/{username}", wrapHandler(gzipUserProfileHandler)).Name("user_profile").Methods("GET") + userRoutes.HandleFunc("/{id}/{username}/follow", UserFollowHandler).Name("user_follow").Methods("GET") + userRoutes.Handle("/{id}/{username}/edit", wrapHandler(gzipUserDetailsHandler)).Name("user_profile_details").Methods("GET") + userRoutes.Handle("/{id}/{username}/edit", wrapHandler(gzipUserProfileFormHandler)).Name("user_profile_edit").Methods("POST") + userRoutes.Handle("/{id}/{username}/apireset", wrapHandler(gzipUserAPIKeyResetHandler)).Name("user_profile_apireset").Methods("GET") + userRoutes.Handle("/notifications", wrapHandler(gzipUserNotificationsHandler)).Name("user_notifications") + userRoutes.HandleFunc("/{id}/{username}/feed", RSSHandler).Name("feed_user") + userRoutes.HandleFunc("/{id}/{username}/feed/{page}", RSSHandler).Name("feed_user_page") + + // Please make EnableSecureCSRF to false when testing locally + if config.EnableSecureCSRF { + userRoutes.Handle("/", csrf.Protect(config.CSRFTokenHashKey)(userRoutes)) + torrentRoutes.Handle("/", csrf.Protect(config.CSRFTokenHashKey)(torrentRoutes)) + torrentViewRoutes.Handle("/", csrf.Protect(config.CSRFTokenHashKey)(torrentViewRoutes)) + } else { + userRoutes.Handle("/", csrf.Protect(config.CSRFTokenHashKey, csrf.Secure(false))(userRoutes)) + torrentRoutes.Handle("/", csrf.Protect(config.CSRFTokenHashKey, csrf.Secure(false))(torrentRoutes)) + torrentViewRoutes.Handle("/", csrf.Protect(config.CSRFTokenHashKey, csrf.Secure(false))(torrentViewRoutes)) + } + + // We don't need CSRF here + api := Router.PathPrefix("/api").Subrouter() + api.Handle("", wrapHandler(gzipAPIHandler)).Methods("GET") + api.Handle("/", wrapHandler(gzipAPIHandler)).Methods("GET") + api.Handle("/{page:[0-9]*}", wrapHandler(gzipAPIHandler)).Methods("GET") + api.Handle("/view/{id}", wrapHandler(gzipAPIViewHandler)).Methods("GET") + api.HandleFunc("/view/{id}", APIViewHeadHandler).Methods("HEAD") + api.HandleFunc("/upload", APIUploadHandler).Methods("POST") + api.HandleFunc("/search", APISearchHandler) + api.HandleFunc("/search/{page}", APISearchHandler) + api.HandleFunc("/update", APIUpdateHandler).Methods("PUT") + // INFO Everything under /mod should be wrapped by wrapModHandler. This make // sure the page is only accessible by moderators + // We don't need CSRF here // TODO Find a native mux way to add a 'prehook' for route /mod Router.HandleFunc("/mod", wrapModHandler(IndexModPanel)).Name("mod_index") Router.HandleFunc("/mod/torrents", wrapModHandler(TorrentsListPanel)).Name("mod_tlist").Methods("GET") @@ -109,7 +137,9 @@ func init() { Router.HandleFunc("/mod/comment/delete", wrapModHandler(CommentDeleteModPanel)).Name("mod_cdelete") Router.HandleFunc("/mod/reassign", wrapModHandler(TorrentReassignModPanel)).Name("mod_treassign").Methods("GET") Router.HandleFunc("/mod/reassign", wrapModHandler(TorrentPostReassignModPanel)).Name("mod_treassign").Methods("POST") - Router.HandleFunc("/mod/api/torrents", wrapModHandler(APIMassMod)).Name("mod_tapi").Methods("POST") + + apiMod := Router.PathPrefix("/mod/api").Subrouter() + apiMod.HandleFunc("/torrents", wrapModHandler(APIMassMod)).Name("mod_tapi").Methods("POST") //reporting a torrent Router.HandleFunc("/report/{id}", ReportTorrentHandler).Methods("POST").Name("torrent_report") From 55867720cb3ca976a7306eeb91216903867f61a2 Mon Sep 17 00:00:00 2001 From: akuma06 Date: Tue, 30 May 2017 00:22:23 +0200 Subject: [PATCH 2/2] Forgot that they also login through the login form So no CSRF protection for login :/ --- router/router.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/router/router.go b/router/router.go index c6125636..574452e9 100755 --- a/router/router.go +++ b/router/router.go @@ -60,6 +60,7 @@ func init() { // For now, no CSRF protection here, as API is not usable for uploads Router.HandleFunc("/upload", UploadHandler).Name("upload") + Router.HandleFunc("/user/login", UserLoginPostHandler).Name("user_login").Methods("POST") torrentViewRoutes := Router.PathPrefix("/view").Subrouter() torrentViewRoutes.Handle("/{id}", wrapHandler(gzipViewHandler)).Methods("GET").Name("view_torrent") @@ -75,7 +76,6 @@ func init() { userRoutes.HandleFunc("/register", UserRegisterFormHandler).Name("user_register").Methods("GET") userRoutes.HandleFunc("/login", UserLoginFormHandler).Name("user_login").Methods("GET") userRoutes.HandleFunc("/register", UserRegisterPostHandler).Name("user_register").Methods("POST") - userRoutes.HandleFunc("/login", UserLoginPostHandler).Name("user_login").Methods("POST") userRoutes.HandleFunc("/logout", UserLogoutHandler).Name("user_logout") userRoutes.Handle("/{id}/{username}", wrapHandler(gzipUserProfileHandler)).Name("user_profile").Methods("GET") userRoutes.HandleFunc("/{id}/{username}/follow", UserFollowHandler).Name("user_follow").Methods("GET")