Improved new account security.

Since chpasswd takes multiple username:password lines
it was possible to change the password of any account:
curl -data "username=attacker&password=%0aroot:omghax" -k https://ctf/new

1 files changed, 14 insertions, 6 deletions

diff --git a/src/ctf_website/views/new.clj b/src/ctf_website/views/new.clj
index 70da5c7..41f2e43 100644
--- a/src/ctf_website/views/new.clj
+++ b/src/ctf_website/views/new.clj
@@ -19,19 +19,27 @@
       [:input {:type "submit"
                :value "Create"}]]]))
 
+(def fail
+  (common/layout
+    [:p "Try a username that doesn't suck"]))
+
+(def good
+  (ring.util.response/redirect "login"))
+
 (defpage
   [:post "/new"] {:keys [username password]}
   (let [adduser (.start (ProcessBuilder. (list "adduser" username)))
         _ (.close (.getOutputStream adduser))
         retval (.waitFor adduser)]
     (if (not (= 0 retval))
-      (common/layout
-        [:p "Try a username that doesn't suck"]
-        [:p (str "result: " retval)])
+      fail ;; this seems to take care of usernames containing : or \n
       (let [chpasswd (.start (ProcessBuilder. (list "chpasswd")))
             out (.getOutputStream chpasswd)
-            _ (.write out (.getBytes (str username ":" password) "UTF-8"))
+            userpass (.getBytes (str username ":" password) "UTF-8")
+            ;; chpasswd seems to only care about \n, though I only tested
+            ;; \n and \r and \0.
+            userpass (remove #(= 0x0a %) userpass)
+            _ (.write out (into-array Byte/TYPE userpass))
             _ (.close out)
             retval (.waitFor chpasswd)]
-        (common/layout
-          [:p (str "result: " retval)])))))
+        good))))