From 45b1bef0919f9115f74b5b232e8fda4c787ba03d Mon Sep 17 00:00:00 2001 From: =?utf8?q?Felix=20D=C3=B6rre?= Date: Thu, 10 Jul 2014 00:08:55 +0200 Subject: [PATCH] Correct csrf-token impl. --- src/org/cacert/gigi/output/Form.java | 23 ++++++-- .../cacert/gigi/pages/account/ChangeForm.java | 4 ++ .../pages/account/ChangePasswordPage.java | 2 +- .../cacert/gigi/pages/main/RegisterPage.java | 20 +++---- src/org/cacert/gigi/pages/main/Signup.java | 4 +- .../cacert/gigi/pages/wot/AssuranceForm.java | 3 +- src/org/cacert/gigi/pages/wot/AssurePage.java | 52 +++++++++---------- 7 files changed, 60 insertions(+), 48 deletions(-) diff --git a/src/org/cacert/gigi/output/Form.java b/src/org/cacert/gigi/output/Form.java index d321fc31..16440cc3 100644 --- a/src/org/cacert/gigi/output/Form.java +++ b/src/org/cacert/gigi/output/Form.java @@ -5,6 +5,7 @@ import java.util.Map; import javax.servlet.ServletRequest; import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpSession; import org.cacert.gigi.Language; import org.cacert.gigi.pages.Page; @@ -13,8 +14,11 @@ import org.cacert.gigi.util.RandomToken; public abstract class Form implements Outputable { String csrf; - public Form() { + public Form(HttpServletRequest hsr) { csrf = RandomToken.generateToken(32); + HttpSession hs = hsr.getSession(); + hs.setAttribute("form/" + getClass().getName() + "/" + csrf, this); + } public abstract boolean submit(PrintWriter out, HttpServletRequest req); @@ -23,7 +27,7 @@ public abstract class Form implements Outputable { public final void output(PrintWriter out, Language l, Map vars) { out.println("
"); outputContent(out, l, vars); - out.print("
"); } @@ -46,7 +50,20 @@ public abstract class Form implements Outputable { } } - public class CSRFError extends Error { + public static T getForm(HttpServletRequest req, Class target) { + String csrf = req.getParameter("csrf"); + if (csrf == null) { + throw new CSRFError(); + } + HttpSession hs = req.getSession(); + if (hs == null) { + throw new CSRFError(); + } + Form f = (Form) hs.getAttribute("form/" + target.getName() + "/" + csrf); + return (T) f; + } + + public static class CSRFError extends Error { } } diff --git a/src/org/cacert/gigi/pages/account/ChangeForm.java b/src/org/cacert/gigi/pages/account/ChangeForm.java index f69dbd11..f200c323 100644 --- a/src/org/cacert/gigi/pages/account/ChangeForm.java +++ b/src/org/cacert/gigi/pages/account/ChangeForm.java @@ -11,6 +11,10 @@ import org.cacert.gigi.output.Form; import org.cacert.gigi.output.Template; public class ChangeForm extends Form { + public ChangeForm(HttpServletRequest hsr) { + super(hsr); + } + private static Template t; static { t = new Template( diff --git a/src/org/cacert/gigi/pages/account/ChangePasswordPage.java b/src/org/cacert/gigi/pages/account/ChangePasswordPage.java index 703bbdc3..1919f544 100644 --- a/src/org/cacert/gigi/pages/account/ChangePasswordPage.java +++ b/src/org/cacert/gigi/pages/account/ChangePasswordPage.java @@ -17,7 +17,7 @@ public class ChangePasswordPage extends Page { @Override public void doGet(HttpServletRequest req, HttpServletResponse resp) throws IOException { - new ChangeForm().output(resp.getWriter(), getLanguage(req), new HashMap()); + new ChangeForm(req).output(resp.getWriter(), getLanguage(req), new HashMap()); } } diff --git a/src/org/cacert/gigi/pages/main/RegisterPage.java b/src/org/cacert/gigi/pages/main/RegisterPage.java index 38c69974..b80429fb 100644 --- a/src/org/cacert/gigi/pages/main/RegisterPage.java +++ b/src/org/cacert/gigi/pages/main/RegisterPage.java @@ -8,6 +8,7 @@ import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; import javax.servlet.http.HttpSession; +import org.cacert.gigi.output.Form; import org.cacert.gigi.pages.Page; public class RegisterPage extends Page { @@ -24,25 +25,16 @@ public class RegisterPage extends Page { PrintWriter out = resp.getWriter(); HashMap vars = new HashMap(); getDefaultTemplate().output(out, getLanguage(req), vars); - Signup s = getForm(req); + Signup s = new Signup(req); s.output(out, getLanguage(req), vars); } - public Signup getForm(HttpServletRequest req) { - HttpSession hs = req.getSession(); - Signup s = (Signup) hs.getAttribute(SIGNUP_PROCESS); - if (s == null) { - s = new Signup(); - hs.setAttribute(SIGNUP_PROCESS, s); - } - return s; - - } - @Override public void doPost(HttpServletRequest req, HttpServletResponse resp) throws IOException { - Signup s = getForm(req); - if (s.submit(resp.getWriter(), req)) { + Signup s = Form.getForm(req, Signup.class); + if (s == null) { + resp.getWriter().println(translate(req, "CSRF token check failed.")); + } else if (s.submit(resp.getWriter(), req)) { HttpSession hs = req.getSession(); hs.setAttribute(SIGNUP_PROCESS, null); resp.getWriter().println( diff --git a/src/org/cacert/gigi/pages/main/Signup.java b/src/org/cacert/gigi/pages/main/Signup.java index 560b2727..04f81149 100644 --- a/src/org/cacert/gigi/pages/main/Signup.java +++ b/src/org/cacert/gigi/pages/main/Signup.java @@ -32,7 +32,8 @@ public class Signup extends Form { Template t; boolean general = true, country = true, regional = true, radius = true; - public Signup() { + public Signup(HttpServletRequest hsr) { + super(hsr); try { t = new Template(new InputStreamReader(Signup.class.getResourceAsStream("Signup.templ"), "UTF-8")); } catch (UnsupportedEncodingException e) { @@ -63,6 +64,7 @@ public class Signup extends Form { vars.put("radius", radius ? " checked=\"checked\"" : ""); vars.put("helpOnNames", String.format(l.getTranslation("Help on Names %sin the wiki%s"), "", "")); + vars.put("csrf", getCSRFToken()); t.output(out, l, vars); } diff --git a/src/org/cacert/gigi/pages/wot/AssuranceForm.java b/src/org/cacert/gigi/pages/wot/AssuranceForm.java index 3743e321..7a49ee1b 100644 --- a/src/org/cacert/gigi/pages/wot/AssuranceForm.java +++ b/src/org/cacert/gigi/pages/wot/AssuranceForm.java @@ -26,7 +26,8 @@ public class AssuranceForm extends Form { templ = new Template(new InputStreamReader(AssuranceForm.class.getResourceAsStream("AssuranceForm.templ"))); } - public AssuranceForm(int assuree) { + public AssuranceForm(HttpServletRequest hsr, int assuree) { + super(hsr); this.assuree = new User(assuree); } diff --git a/src/org/cacert/gigi/pages/wot/AssurePage.java b/src/org/cacert/gigi/pages/wot/AssurePage.java index 70b5985e..9c29044b 100644 --- a/src/org/cacert/gigi/pages/wot/AssurePage.java +++ b/src/org/cacert/gigi/pages/wot/AssurePage.java @@ -10,13 +10,11 @@ import java.util.HashMap; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; -import javax.servlet.http.HttpSession; - import org.cacert.gigi.User; import org.cacert.gigi.database.DatabaseConnection; import org.cacert.gigi.output.DateSelector; +import org.cacert.gigi.output.Form; import org.cacert.gigi.output.Template; -import org.cacert.gigi.output.Form.CSRFError; import org.cacert.gigi.pages.LoginPage; import org.cacert.gigi.pages.Page; import org.cacert.gigi.util.Notary; @@ -24,7 +22,6 @@ import org.cacert.gigi.util.Notary.AssuranceResult; public class AssurePage extends Page { public static final String PATH = "/wot/assure"; - public static final String SESSION = "/wot/assure/FORM"; DateSelector ds = new DateSelector("day", "month", "year"); Template t; @@ -40,22 +37,10 @@ public class AssurePage extends Page { PrintWriter out = resp.getWriter(); String pi = req.getPathInfo().substring(PATH.length()); if (pi.length() > 1) { - User myself = LoginPage.getUser(req); int mid = Integer.parseInt(pi.substring(1)); - AssuranceResult check = Notary.checkAssuranceIsPossible(myself, new User(mid)); - if (check != AssuranceResult.ASSURANCE_SUCCEDED) { - out.println(translate(req, check.getMessage())); - return; - } - HttpSession hs = req.getSession(); - AssuranceForm form = (AssuranceForm) hs.getAttribute(SESSION); - if (form == null || form.assuree.getId() != mid) { - form = new AssuranceForm(mid); - hs.setAttribute(SESSION, form); - } + AssuranceForm form = new AssuranceForm(req, mid); + outputForm(req, out, mid, form); - form.output(out, getLanguage(req), new HashMap()); - ; } else { HashMap vars = new HashMap(); vars.put("DoB", ds); @@ -63,6 +48,20 @@ public class AssurePage extends Page { } } + private void outputForm(HttpServletRequest req, PrintWriter out, int mid, AssuranceForm form) { + User myself = LoginPage.getUser(req); + AssuranceResult check = Notary.checkAssuranceIsPossible(myself, new User(mid)); + if (check != AssuranceResult.ASSURANCE_SUCCEDED) { + out.println(translate(req, check.getMessage())); + return; + } + if (form == null || form.assuree.getId() != mid) { + form = new AssuranceForm(req, mid); + } + + form.output(out, getLanguage(req), new HashMap()); + } + @Override public void doPost(HttpServletRequest req, HttpServletResponse resp) throws IOException { PrintWriter out = resp.getWriter(); @@ -71,26 +70,23 @@ public class AssurePage extends Page { User myself = LoginPage.getUser(req); int mid = Integer.parseInt(pi.substring(1)); if (mid == myself.getId()) { - out.println("Cannot assure myself."); + out.println(translate(req, "Cannot assure myself.")); return; } - AssuranceForm form = (AssuranceForm) req.getSession().getAttribute(SESSION); - if (form == null) { - out.println("No form found. This is an Error. Fill in the form again."); + AssuranceForm form = Form.getForm(req, AssuranceForm.class); + if (mid != form.assuree.getId()) { return; } - try { - form.submit(out, req); - } catch (CSRFError e) { - resp.sendError(500, "CSRF Failed"); - out.println(translate(req, "CSRF Token failed.")); + if (form.submit(out, req)) { + out.println(translate(req, "Assurance complete.")); + } else { + outputForm(req, resp.getWriter(), mid, form); } return; } - System.out.println("searching for"); ResultSet rs = null; try { PreparedStatement ps = DatabaseConnection.getInstance().prepare( -- 2.39.2