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

Ascending order - Click to sort in descending order
Tim McCormack made changes -
Field Original Value New Value
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. 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.
Tim McCormack made changes -
Affects Version/s Release 1.4 [ 10040 ]
Affects Version/s Release 1.1 [ 10036 ]
Affects Version/s Release 1.2 [ 10037 ]
Hide
Permalink
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.
Tim McCormack made changes -
Attachment 0001-CLJ-904-Note-about-read-eval-on-docstrings-for-reade.patch [ 11673 ]
Andy Fingerhut made changes -
Patch Code [ 10001 ]
Hide
Permalink
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.
Tim McCormack made changes -
Attachment 0001-CLJ-904-Note-about-read-eval-on-docstrings-for-reade.patch [ 11673 ]
Hide
Permalink
Tim McCormack added a comment -

Oops! Replaced the patch.

Show
Tim McCormack added a comment - Oops! Replaced the patch.
Tim McCormack made changes -
Attachment 0002-CLJ-904-Note-about-read-eval-on-docstrings-for-reade.patch [ 11679 ]
Hide
Permalink
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.
Christopher Redinger made changes -
Approval Screened [ 10004 ]
Priority Major [ 3 ] Trivial [ 5 ]
Hide
Permalink
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
Permalink
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
Permalink
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
Permalink
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.
Rich Hickey made changes -
Resolution Completed [ 1 ]
Approval Screened [ 10004 ] Vetted [ 10003 ]
Status Open [ 1 ] Resolved [ 5 ]
Stuart Halloway made changes -
Status Resolved [ 5 ] Closed [ 6 ]

People

Vote (6)
Watch (5)

Dates

  • Created:
    Updated:
    Resolved:
Atlassian JIRA (v4.4#649-r158309) | Report a problem
Powered by a free Atlassian JIRA open source license for Clojure. Try JIRA - bug tracking software for your team.