Skip to content
Permalink
Browse files Browse the repository at this point in the history
Merge pull request from GHSA-cp4w-6x4w-v2h5
Treat a backslash in the authority section as part of the path
  • Loading branch information
plexus committed Mar 27, 2023
2 parents d3355fc + 67063ed commit f46db3e
Show file tree
Hide file tree
Showing 3 changed files with 30 additions and 11 deletions.
7 changes: 3 additions & 4 deletions CHANGELOG.md
@@ -1,10 +1,9 @@
# Unreleased

## Added

## Fixed

## Changed
- Treat a backslash in the authority section as a delimiter which starts the
path section (CVE-2023-28628, with thanks to @luigigubello for the report)

# 1.13.95 (2022-01-28 / a9cbeff)

Expand Down Expand Up @@ -108,4 +107,4 @@ not take into account utf-16 encoding.

## Added

- Initial release, public vars: `uri`, `join`, `coerce`, `parse`, `edn-readers`
- Initial release, public vars: `uri`, `join`, `coerce`, `parse`, `edn-readers`
4 changes: 2 additions & 2 deletions src/lambdaisland/uri.cljc
Expand Up @@ -4,8 +4,8 @@
[lambdaisland.uri.normalize :as normalize])
#?(:clj (:import clojure.lang.IFn)))

(def uri-regex #?(:clj #"\A(([^:/?#]+):)?(//([^/?#]*))?([^?#]*)?(\?([^#]*))?(#(.*))?\z"
:cljs #"^(([^:/?#]+):)?(//([^/?#]*))?([^?#]*)?(\?([^#]*))?(#(.*))?$"))
(def uri-regex #?(:clj #"\A(([^:/?#]+):)?(//([^/?#\\]*))?([^?#]*)?(\?([^#]*))?(#(.*))?\z"
:cljs #"^(([^:/?#]+):)?(//([^/?#\\]*))?([^?#]*)?(\?([^#]*))?(#(.*))?$"))
(def authority-regex #?(:clj #"\A(([^:]*)(:(.*))?@)?([^:]*)(:(\d*))?\z"
:cljs #"^(([^:]*)(:(.*))?@)?([^:]*)(:(\d*))?$"))

Expand Down
30 changes: 25 additions & 5 deletions test/lambdaisland/uri_test.cljc
Expand Up @@ -181,8 +181,28 @@

(tc/defspec query-string-round-trips 100
(prop/for-all [q query-map-gen]
(let [res (-> q
uri/map->query-string
uri/query-string->map)]
(or (and (empty? q) (empty? res)) ;; (= nil {})
(= q res)))))
(let [res (-> q
uri/map->query-string
uri/query-string->map)]
(or (and (empty? q) (empty? res)) ;; (= nil {})
(= q res)))))

(deftest backslash-in-authority-test
;; A backslash is not technically a valid character in a URI (see RFC 3986
;; section 2), and so should always be percent encoded. The problem is that
;; user-facing software (e.g. browsers) rarely if ever rejects invalid
;; URIs/URLs, leading to ad-hoc rules about how to map the set of invalid URIs
;; to valid URIs. All modern browsers now interpret a backslash as a forward
;; slash, which changes the interpretation of the URI. For this test (and
;; accompanying patch) we only care about the specific case of a backslash
;; appearing inside the authority section, since this authority or _origin_ is
;; regularly used to inform security policies, e.g. to check if code served
;; from a certain origin has access to resources with the same origin. In this
;; case we partially mimic what browsers do, by treating the backslash as a
;; delimiter which starts the path section, even though we don't replace it
;; with a forward slash, but leave it as-is in the parsed result.
(let [{:keys [host path user]}
(uri/uri "https://example.com\\@gaiwan.co")]
(is (= "example.com" host))
(is (= nil user))
(is (= "\\@gaiwan.co" path))))

0 comments on commit f46db3e

Please sign in to comment.