From d8ca1d70b7a52404211d31a21845b76c1ce9f631 Mon Sep 17 00:00:00 2001 From: Bat Date: Wed, 19 Sep 2018 18:13:07 +0100 Subject: [PATCH] Fix CSRF issues GET routes are not protected against CSRF. This commit changes the needed URLs to POST and replace simple links with forms. Thanks @fdb-hiroshima for noticing it! --- po/gl.po | 21 ++++++++++++++------- src/routes/instance.rs | 4 ++-- src/routes/medias.rs | 4 ++-- src/routes/posts.rs | 2 +- src/routes/user.rs | 6 +++--- static/css/main.css | 27 +++++++++++++++++++++++++++ templates/instance/list.html.tera | 8 ++++---- templates/instance/users.html.tera | 6 +++--- templates/medias/details.html.tera | 8 ++++++-- templates/posts/details.html.tera | 10 ++++++---- templates/users/edit.html.tera | 4 +++- templates/users/header.html.tera | 8 +++++--- 12 files changed, 76 insertions(+), 32 deletions(-) diff --git a/po/gl.po b/po/gl.po index 213ecff8..5977ec7a 100644 --- a/po/gl.po +++ b/po/gl.po @@ -325,7 +325,8 @@ msgstr[1] "{{ count }} autoras en este blog: " msgid "Login or use your Fediverse account to interact with this article" msgstr "" -"Conéctese ou utilice a súa conta no fediverso para interactuar con este artigo" +"Conéctese ou utilice a súa conta no fediverso para interactuar con este " +"artigo" msgid "Optional" msgstr "Opcional" @@ -486,7 +487,6 @@ msgstr "Descrición" msgid "Content warning" msgstr "Aviso sobre o contido" - msgid "File" msgstr "Ficheiro" @@ -496,7 +496,8 @@ msgstr "Enviar" msgid "" "Sorry, but registrations are closed on this instance. Try to find another one" msgstr "" -"Lamentámolo, pero o rexistro está pechado en esta instancia. Intente atopar outra" +"Lamentámolo, pero o rexistro está pechado en esta instancia. Intente atopar " +"outra" msgid "Subtitle" msgstr "Subtítulo" @@ -553,9 +554,10 @@ msgid "" "Something is wrong with your CSRF token. Make sure cookies are enabled in " "you browser, and try reloading this page. If you continue to see this error " "message, please report it." -msgstr "Hai un problema co seu testemuño CSRF. Asegúrese de ter as cookies activadas " -"no navegador, e recargue a páxina. Si persiste o aviso de este fallo, " -" informe por favor." +msgstr "" +"Hai un problema co seu testemuño CSRF. Asegúrese de ter as cookies activadas " +"no navegador, e recargue a páxina. Si persiste o aviso de este fallo, " +"informe por favor." msgid "Administration of {{ instance.name }}" msgstr "Administración de {{ instance_name }}" @@ -600,7 +602,12 @@ msgid "Delete your account" msgstr "Eliminar a súa conta" msgid "Sorry, but as an admin, you can't leave your instance." -msgstr "Lamentámolo, pero como administradora, non pode deixar a súa instancia." +msgstr "" +"Lamentámolo, pero como administradora, non pode deixar a súa instancia." msgid "Users" msgstr "Usuarias" + +#, fuzzy +msgid "This post isn't published yet." +msgstr "Esto é un borrador, non publicar por agora." diff --git a/src/routes/instance.rs b/src/routes/instance.rs index 25f56bb5..0db72f20 100644 --- a/src/routes/instance.rs +++ b/src/routes/instance.rs @@ -156,7 +156,7 @@ fn admin_instances_paginated(admin: Admin, conn: DbConn, page: Page) -> Template })) } -#[get("/admin/instances//block")] +#[post("/admin/instances//block")] fn toggle_block(_admin: Admin, conn: DbConn, id: i32) -> Redirect { if let Some(inst) = Instance::get(&*conn, id) { inst.toggle_block(&*conn); @@ -183,7 +183,7 @@ fn admin_users_paginated(admin: Admin, conn: DbConn, page: Page) -> Template { })) } -#[get("/admin/users//ban")] +#[post("/admin/users//ban")] fn ban(_admin: Admin, conn: DbConn, id: i32) -> Redirect { User::get(&*conn, id).map(|u| u.delete(&*conn)); Redirect::to(uri!(admin_users)) diff --git a/src/routes/medias.rs b/src/routes/medias.rs index fa9197d4..011567e0 100644 --- a/src/routes/medias.rs +++ b/src/routes/medias.rs @@ -97,14 +97,14 @@ fn details(id: i32, user: User, conn: DbConn) -> Template { })) } -#[get("/medias//delete")] +#[post("/medias//delete")] fn delete(id: i32, _user: User, conn: DbConn) -> Redirect { let media = Media::get(&*conn, id).expect("Media to delete not found"); media.delete(&*conn); Redirect::to(uri!(list)) } -#[get("/medias//avatar")] +#[post("/medias//avatar")] fn set_avatar(id: i32, user: User, conn: DbConn) -> Redirect { let media = Media::get(&*conn, id).expect("Media to delete not found"); user.set_avatar(&*conn, media.id); diff --git a/src/routes/posts.rs b/src/routes/posts.rs index 29ce308c..20ad1bc0 100644 --- a/src/routes/posts.rs +++ b/src/routes/posts.rs @@ -338,7 +338,7 @@ fn create(blog_name: String, data: LenientForm, user: User, conn: D } } -#[get("/~///delete")] +#[post("/~///delete")] fn delete(blog_name: String, slug: String, conn: DbConn, user: User, worker: State>>) -> Redirect { let post = Blog::find_by_fqn(&*conn, blog_name.clone()) .and_then(|blog| Post::find_by_slug(&*conn, slug.clone(), blog.id)); diff --git a/src/routes/user.rs b/src/routes/user.rs index 3deb2574..42ac6230 100644 --- a/src/routes/user.rs +++ b/src/routes/user.rs @@ -118,7 +118,7 @@ fn dashboard_auth() -> Flash { ) } -#[get("/@//follow")] +#[post("/@//follow")] fn follow(name: String, conn: DbConn, user: User, worker: Worker) -> Redirect { let target = User::find_by_fqn(&*conn, name.clone()).unwrap(); if let Some(follow) = follows::Follow::find(&*conn, user.id, target.id) { @@ -138,7 +138,7 @@ fn follow(name: String, conn: DbConn, user: User, worker: Worker) -> Redirect { Redirect::to(uri!(details: name = name)) } -#[get("/@//follow", rank = 2)] +#[post("/@//follow", rank = 2)] fn follow_auth(name: String) -> Flash { utils::requires_login( "You need to be logged in order to follow someone", @@ -225,7 +225,7 @@ fn update(_name: String, conn: DbConn, user: User, data: LenientForm/delete")] +#[post("/@//delete")] fn delete(name: String, conn: DbConn, user: User, mut cookies: Cookies) -> Redirect { let account = User::find_by_fqn(&*conn, name.clone()).unwrap(); if user.id == account.id { diff --git a/static/css/main.css b/static/css/main.css index 948ad303..85334afc 100644 --- a/static/css/main.css +++ b/static/css/main.css @@ -472,6 +472,33 @@ main .article-meta .tags li a { width: initial; } +/** Inline forms (containing only CSRF token and a , for protected links) **/ + +form.inline { + display: inline; + margin: 0px; + padding: 0px; + width: auto; +} + +form.inline input[type="submit"] { + display: inline-block; + color: #7765E3; + cursor: pointer; + font-size: 1em; + width: auto; +} + +form.inline input[type="submit"]:not(.button) { + margin: 0; + border: none; +} + +form.inline input[type="submit"]:not(.button) { + background: transparent; + color: #7765E3; +} + /* Button & Submit */ .button, input[type="submit"], button { diff --git a/templates/instance/list.html.tera b/templates/instance/list.html.tera index e792ed1c..878cc085 100644 --- a/templates/instance/list.html.tera +++ b/templates/instance/list.html.tera @@ -18,13 +18,13 @@ {{ instance.public_domain }}

{% if not instance.local %} - +
{% if instance.blocked %} - {{ "Unblock" | _ }} + {% else %} - {{ "Block" | _ }} + {% endif %} - +
{% endif %} {% endfor %} diff --git a/templates/instance/users.html.tera b/templates/instance/users.html.tera index fa8fcec6..ff00a86e 100644 --- a/templates/instance/users.html.tera +++ b/templates/instance/users.html.tera @@ -19,9 +19,9 @@ @{{ user.username }}

{% if not user.is_admin %} - - {{ "Ban" | _ }} - +
+ +
{% endif %} {% endfor %} diff --git a/templates/medias/details.html.tera b/templates/medias/details.html.tera index 4ba8857d..9359de47 100644 --- a/templates/medias/details.html.tera +++ b/templates/medias/details.html.tera @@ -24,8 +24,12 @@ {{ media.md }} {% endblock content %} diff --git a/templates/posts/details.html.tera b/templates/posts/details.html.tera index 449e8827..a6815e88 100644 --- a/templates/posts/details.html.tera +++ b/templates/posts/details.html.tera @@ -12,7 +12,7 @@ {% block content %}

{{ article.post.title }}

{{ article.post.subtitle }}

-

+

{{ "Written by {{ link_1 }}{{ url }}{{ link_2 }}{{ name | escape }}{{ link_3 }}" | _( link_1='" ) - }} + }} — {{ date | date(format="%B %e, %Y") }} {% if is_author %} — {{ "Edit" | _ }} — - {{ "Delete this article" | _ }} +
+ +
{% endif %} {% if not article.post.published %} {{ "Draft" }} {% endif %} -

+
{{ article.post.content | safe }}
diff --git a/templates/users/edit.html.tera b/templates/users/edit.html.tera index 980d2ba1..42c55856 100644 --- a/templates/users/edit.html.tera +++ b/templates/users/edit.html.tera @@ -25,7 +25,9 @@

{{ "Danger zone" | _ }}

{{ "Be very careful, any action taken here can't be cancelled." | _ }} {% if not account.is_admin %} -

{{ "Delete your account" | _ }}

+
+ +
{% else %}

{{ "Sorry, but as an admin, you can't leave your instance." | _ }}

{% endif %} diff --git a/templates/users/header.html.tera b/templates/users/header.html.tera index eded30ee..6e7ce73c 100644 --- a/templates/users/header.html.tera +++ b/templates/users/header.html.tera @@ -23,16 +23,18 @@ {% if is_remote %} - {{ "Open on {{ instance_url }}" | _(instance_url=instance_url) }} + {{ "Open on {{ instance_url }}" | _(instance_url=instance_url) }} {% endif %} {% set not_self = not is_self %} {% if not_self and (account is defined) %} +
{% if follows %} - {{ "Unfollow" | _ }} + {% else %} - {{ "Follow" | _ }} + {% endif %} +
{% endif %}