Clojure

Document *read-eval* in read, read-string

Details

  • Type: Defect Defect
  • Status: Closed Closed
  • Priority: Trivial Trivial
  • Resolution: Completed
  • Affects Version/s: Release 1.1, Release 1.2, Release 1.3, Release 1.4
  • Fix Version/s: None
  • Component/s: None
  • Labels:
    None
  • Patch:
    Code
  • Approval:
    Vetted

Description

Even though the #=() reader syntax is "unofficial", *read-eval* should be documented in the appropriate API functions – this is a serious security problem for anyone accepting serialized Clojure data structures. E.g., a system service reading a config file, a server accepting an API request.

Activity

Hide
Tim McCormack added a comment -

My goodness, I entirely neglected to attach a patch for this! Well, here it is, short and sweet.

Show
Tim McCormack added a comment - My goodness, I entirely neglected to attach a patch for this! Well, here it is, short and sweet.
Hide
Andy Fingerhut added a comment -

Tim, I'm pretty sure that read-line's behavior is safe regardless of the value of read-eval. It only reads characters from the stream without interpretation or evaluation, and returns them as a string. If so, adding the warning to read-line's doc string seems wrong.

Show
Andy Fingerhut added a comment - Tim, I'm pretty sure that read-line's behavior is safe regardless of the value of read-eval. It only reads characters from the stream without interpretation or evaluation, and returns them as a string. If so, adding the warning to read-line's doc string seems wrong.
Hide
Tim McCormack added a comment -

Oops! Replaced the patch.

Show
Tim McCormack added a comment - Oops! Replaced the patch.
Hide
Christopher Redinger added a comment -

Patch applies cleanly and adds a useful message.

Show
Christopher Redinger added a comment - Patch applies cleanly and adds a useful message.
Hide
Steve Miner added a comment -

See also discussion on the mailing list:
https://groups.google.com/forum/?fromgroups=#!topic/clojure/qUk-bM0JSGc

Several people who care about safety think that *read-eval* should be false by default. Documentation does not make up for a security hole.

Show
Steve Miner added a comment - See also discussion on the mailing list: https://groups.google.com/forum/?fromgroups=#!topic/clojure/qUk-bM0JSGc Several people who care about safety think that *read-eval* should be false by default. Documentation does not make up for a security hole.
Hide
Steve Miner added a comment -

It would be useful to document the interaction between *print-dup* and *read-eval* as well. In most cases, if you use *print-dup* true to preserve exact types, you'll want to bind *read-eval* true to read them. Code that depends on their default values is brittle.

Show
Steve Miner added a comment - It would be useful to document the interaction between *print-dup* and *read-eval* as well. In most cases, if you use *print-dup* true to preserve exact types, you'll want to bind *read-eval* true to read them. Code that depends on their default values is brittle.
Hide
Andy Fingerhut added a comment -

See also the newer ticket CLJ-1153, which has a patch to change read-eval default value to false.

Show
Andy Fingerhut added a comment - See also the newer ticket CLJ-1153, which has a patch to change read-eval default value to false.
Hide
Andy Fingerhut added a comment -

With the recent changes Rich has done to add read-edn and read-edn-string, he has also updated the doc strings of read and read-string to call out the potential security dangers, emphasize that they are intended for use only in reading code and/or data from trusted sources, and to point to the safer read-edn and read-edn-string for data interchange purposes.

The purpose of this ticket seems to be satisfied with those commits.

Show
Andy Fingerhut added a comment - With the recent changes Rich has done to add read-edn and read-edn-string, he has also updated the doc strings of read and read-string to call out the potential security dangers, emphasize that they are intended for use only in reading code and/or data from trusted sources, and to point to the safer read-edn and read-edn-string for data interchange purposes. The purpose of this ticket seems to be satisfied with those commits.

People

Vote (6)
Watch (5)

Dates

  • Created:
    Updated:
    Resolved: