Clojure

Avoid using keywords as sentinel values in transducers

Details

  • Type: Defect Defect
  • Status: Open Open
  • Priority: Minor Minor
  • Resolution: Unresolved
  • Affects Version/s: Release 1.7, Release 1.8, Release 1.9
  • Fix Version/s: None
  • Component/s: None
  • Environment:
    All environments

Description

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.).

Activity

Hide
Rick Moynihan added a comment -

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.

Show
Rick Moynihan added a comment - 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.
Hide
Rick Moynihan added a comment - - edited

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

Show
Rick Moynihan added a comment - - edited Also thanks to @reborg and @bronsa on #clojure-uk for sharing the above snippet.
Hide
Alex Miller added a comment -

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

How is this a security risk?

Show
Alex Miller added a comment - Rick - I added you to the necessary groups for edit privileges. How is this a security risk?
Hide
Rick Moynihan added a comment - - edited

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.

Show
Rick Moynihan added a comment - - edited 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.

People

Vote (0)
Watch (0)

Dates

  • Created:
    Updated: