Avoid using keywords as sentinel values in transducers

Question

The use of keywords as sentinels in transducers could in rare circumstances expose some applications to bugs and potential security risks.

(sequence (partition-by keyword) ["1" "none" "2" "clojure.core/none" "3" "4"]) ;(["1"] ["none"] ["2"] ["clojure.core/none" "3"] ["4"])

Ideally a private or local value that cannot be injected into the functions domain should be used instead, e.g. {{(Object.)}}.

Answer 1

Comment made by: rickmoynihan

Apologies for messing up the formatting on the snippet, I also didn't mean to assign this as major. Though I can't seem to edit it, perhaps someone else can.

Answer 2

Comment made by: rickmoynihan

Also thanks to @reborg and @bronsa on #clojure-uk for sharing the above snippet.

Answer 3

Comment made by: alexmiller

Rick - I added you to the necessary groups for edit privileges.

How is this a security risk?

Answer 4

Comment made by: rickmoynihan

Thanks for upping my privileges Alex.

I didn't mean to over egg the pudding, but perhaps I ended up doing it all the same! :-)

My reasoning was that if you're using a transducer to map/reduce over values taken from user input, then an attacker could cause a loop to abort earlier than expected; which might then have other consequences. I did't have a specific attack vector in mind, but I'd seen also the sentinel ::halt and had imagined that a user could inject "clojure.core/halt" into a HTTP header. If ring has a middleware to keywordize keys then you have the sentinel value for a transducer injected by a user, and that sentinel could then potentially be used to shortcut the validation/encoding/escaping etc of other fields.

However I've looked a bit more at the implementation of {{halt-when}} and other transducers and it seems the key isn't ever mixed with user data; and that ::none is used to indicate the first pass over by some transducer functions. So it looks like I was mistaken. I doubt now that this is worth resolving.

Answer 5

Reference: https://clojure.atlassian.net/browse/CLJ-2312 (reported by rickmoynihan)