Skip to content

Commit a72af40

Browse files
committed
Prevent null character input for project browsing
This isn't valid input, and is someone fuzzing the API.
1 parent f135c83 commit a72af40

File tree

3 files changed

+50
-17
lines changed

3 files changed

+50
-17
lines changed

src/clojars/web.clj

Lines changed: 18 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@
2222
[clojars.routes.user :as user]
2323
[clojars.routes.verify :as verify]
2424
[clojars.web.browse :refer [browse]]
25-
[clojars.web.common :refer [html-doc]]
25+
[clojars.web.common :as common :refer [html-doc]]
2626
[clojars.web.dashboard :refer [dashboard index-page]]
2727
[clojars.web.safe-hiccup :refer [raw]]
2828
[clojars.web.search :as search]
@@ -36,18 +36,19 @@
3636
[ring.middleware.not-modified :refer [wrap-not-modified]]
3737
[ring.util.response :refer [bad-request content-type]]))
3838

39-
(defn try-parse-page
39+
(defn- try-parse-page
4040
"Will throw a targeted error if maybe-page doesn't parse as an integer."
4141
[maybe-page]
42-
(try
43-
(Integer/parseInt maybe-page)
44-
(catch Exception _
45-
(throw (ex-info
46-
"page must be an integer"
47-
{:report? false
48-
:title "Bad Request"
49-
:error-message "The page query parameter must be an integer."
50-
:status 400})))))
42+
(when maybe-page
43+
(try
44+
(Integer/parseInt maybe-page)
45+
(catch Exception _
46+
(throw (ex-info
47+
"page must be an integer"
48+
{:report? false
49+
:title "Bad Request"
50+
:error-message "The page query parameter must be an integer."
51+
:status 400}))))))
5152

5253
(defn- main-routes
5354
[{:as _system :keys [db event-emitter hcaptcha mailer search stats]}]
@@ -60,15 +61,15 @@
6061
(index-page db stats %))))
6162
(GET "/search" {:keys [params]}
6263
(try-account
63-
#(let [validated-params (if (:page params)
64-
(assoc params :page (try-parse-page (:page params)))
65-
params)]
64+
#(let [validated-params (-> params
65+
(update :page try-parse-page))]
6666
(search/search search % validated-params))))
6767
(GET "/projects" {:keys [params]}
6868
(try-account
69-
#(let [validated-params (if (:page params)
70-
(assoc params :page (try-parse-page (:page params)))
71-
params)]
69+
#(let [validated-params
70+
(-> params
71+
(update :from (partial common/check-no-null-bytes "from"))
72+
(update :page try-parse-page))]
7273
(browse db % validated-params))))
7374
(GET "/security" []
7475
(try-account

src/clojars/web/common.clj

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -459,3 +459,18 @@
459459
(for [[label input] label-input-pairs]
460460
[:tr [:td label] [:td input]])]
461461
extra-inputs]))
462+
463+
(defn check-no-null-bytes
464+
"Will throw a targeted error if s contains 0x00, as postgres will not allow
465+
the null byte in strings."
466+
[param-name ^String s]
467+
(if (and s
468+
(>= (.indexOf s 0x00) 0))
469+
(throw (ex-info
470+
"value must be a UTF-8 string"
471+
{:report? false
472+
:title "Bad Request"
473+
:error-message (format "The %s parameter must not contain null bytes."
474+
param-name)
475+
:status 400}))
476+
s))

test/clojars/integration/web_test.clj

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -106,3 +106,20 @@
106106
[:div enlive/first-of-type]
107107
[:a enlive/first-of-type]]
108108
(has (text? "tester/tester123a")))))
109+
110+
(deftest browse-page-rejects-invalid-utf-jump
111+
(help/add-verified-group "test-user" "tester")
112+
(doseq [i (range 100 125)]
113+
(db/add-jar
114+
help/*db*
115+
"test-user"
116+
{:name (str "tester" i "a") :group "tester" :version "0.1" :description "Huh" :authors ["Zz"]}))
117+
(let [{:keys [response]}
118+
(-> (session (help/app))
119+
(visit "/projects")
120+
(fill-in "Enter a few letters..." (String. (byte-array [0x00])))
121+
(press "Jump"))]
122+
(is (match?
123+
{:status 400
124+
:body #"The from parameter must not contain null bytes"}
125+
response))))

0 commit comments

Comments
 (0)