;; especially since read-string will be fixed in clojure 1.5 so that
;; binding *read-eval* to false around it will make it safe, as far as
In clojure 1.5 with read-eval bound to false, the reader is still unsafe for malicious input. Since the Clojure reader must be able to construct arbitrary Java objects via their constructors, any constructor can be called from the reader even with read-eval bound to false.
Glad I ran across this this week- I would have most likely ended up with a security hole in an app otherwise. (I had thought read-eval=false was an adequate safeguard...)
Lisps separate the notion of reading strings into data structures, from evaluating those data structures. So it would be easy to assume that read-string was safe to use, but there were actually some lesser-known features in the Clojure reader that allowed code execution.
You're missing that the whole point here is that when read-eval is set to false then, precisely, you're making your intent very clear: you do NOT want to execute code you didn't write.
The problem is that there's a hole due to Java constructors being called (even when read-eval is set to false).
;; The particular issue of executing arbitrary Java constructors used
;; in the examples above no longer works in Clojure 1.5 when
;; *read-eval* is false. Even so, you SHOULD NEVER USE
;; clojure.core/read or clojure.core/read-string for reading untrusted
;; data. Use an edn reader or a different data serialization format.
From that comment it seems like the Java constructor loophole has been closed. With that gone, I don't understand why the reader is still dangerous. What am I missing?
I was wondering this too, and deliberately tried to break something using the clojure 1.5 reader with read-eval false. (Makes a change from building things all day!).
There's a huge IRC log that explains the reasoning here. Mostly, it comes down to: you can't trust that libraries haven't changed read-eval back, which means that now instead of being assured that a certain fn can never be harmful, you would need to audit all libraries you use. It's the assumption itself (that read and read-string are safe) what makes it unsafe...
It's the same issue as long as you consider that any evaluation of unwanted code is the same issue and if you consider that what happens here with the Java constructors is really "eval'ing" code.
Here I'd say it's a bit different: it's kinda the whole of having read-eval set to true to be able to evaluate code. So when read-eval is set to false and yet rogue code happens to be called there's a serious issue. Basically the issue is that under the hood some Java constructors seems to be called and, if I understand the issue correctly, by knowing how Clojure works you can exploit Clojure / the JVM by crafting precisely what you want to pass to Java constructors (which shouldn't be called in the first place).
Here as I understand it it's not an "eval" exploit per se: it's Java constructors which are passed carefully crafted input.
This is read-string, not read-line. It doesn't mean read a string but to read from a string. It reads the first clojure expression from a string.
It isn't supposed to be safe, though it can be tempting to treat it as though it's safe.
- It's similar to the YAML reader, in that Java objects can be created. See "deftype, defrecord, and constructor calls" here: http://clojure.org/reader
- On the positive side for the clojure reader, it isn't widely described as a serialization format and people are less inclined to use it for user input. This is a pretty important difference.
- On the negative side, you can not only instantiate objects, but you can execute code. This is what the example in the article does. You can disable that by setting read-eval.
- Also on the negative side, it lacks something quite as secure as YAML's safe_load. You can tell it not to evaluate code, but there isn't yet a way to disable instantiating objects.
Not really, read-line is more or less equivalent to python's input method. It reads raw input and tries to evaluate it. Passing false makes it safer for reading data structures, but it will still try to execute Java constructors so that your data is correctly initialized. To be safe you need to use other methods i.e. edn(extensible data notation).
edn limits what is allowed to a safe subset of what read-line will allow, all of the base clojure types and collection types (maps, lists, sets, vectors) are allowed, Java isn't.
Not quite. It reads raw input and tries to build a syntax tree from it. Unfortunately reader macros in clojure allow all sorts of objects to form part of said syntax tree and arbitrary-ish code to run at parse time to instantiate them. But the main job of the reader isn't to evaluate code.
As more people start adopting Lispy languages, it will be important to build up a cultural awareness of what's dangerous. Things like not evaling code is obvious enough (I'd hope), but avoiding read less so. That seems less like "avoid eval" and more like "avoid gets".
It's true that in general, people need to understand what is dangerous in each language they use.
However, as far as I can tell, the problem here has nothing to do with homoiconicity. The "get string" vulnerability, as you call it, comes out of Clojure's (excellent) JVM support, and not from the Lisp side of its family tree. A "read" is not just a read only because reading Java means creating and compiling arbitrary classes and objects. That, in turn, is largely because Java is not homoiconic.
Keep in mind that this is a Clojure-specific issue. In Common Lisp, eval-suppressing macros should prevent this sort of thing (but I would not rely on that -- it is easy to accidentally create a reader macro that fails to check those variables).
Personally, I would only use something like read if I were reading a configuration file i.e. something that only someone with privileged access can modify. I would not even consider using read for arbitrary, untrustworthy data; it is just silly in this day and age. It may make the code look prettier, but the fact that arbitrary data structures can be created and the potential to execute arbitrary code is enough to keep me away.
> That seems less like "avoid eval" and more like "avoid gets".
Assuming that by `gets` you mean Ruby's `gets`, this is not quite right. `read` (and `read-string`) are not just generic IO functions. They are parsers for Clojure code. There are plenty of other varied forms of reading input to a string, but `read` does more.
Sorry, by "gets" I meant gets(3) from C, which probably seems like a perfectly valid way to read input to a new programmer, but is a sure way to give yourself a buffer overflow. My point was that some innocuous-seeming functions are traps, and it's important that knowledge of those traps become part of a language's cultural knowledge, in hopes that it reaches more developers.
But whether Ruby or C, I take your point that Clojure's read is not just an IO function. It's not really the IO-ness of gets that I'm trying to parallel, but its apparent safety. But perhaps to a Clojure programmer, read is not as surprising as gets because it's more clearly an evaling call. Do you think that'd be clear to a first-year Clojure dev?
I can't really speak to what is clear or not for a first-year Clojure dev. When I first learned Clojure, I had already dabbled in Lisps and Schemes for a while. The very first time I ran across `read`, probably in Scheme, it was obvious to me that it was special. Did I understand and appreciate the security implications? Probably not.
Hopefully the security issues will be clear to a new Clojure dev since at least the ClojureDocs site steers you away from read and towards EDN. Maybe the official docs should be more stern and clear about it, but they do make a note of the security issue: http://clojure.github.com/clojure/clojure.core-api.html#cloj...
You feel vindicated over this vacuous comment article? Let me sum it up for you: "It turns out if I set a safety variable, then unset it in a way that most of my audience won't recognize as assignment, I get votes up on hackernews."
Cultural awareness of read-time evaluation is as frustratingly absent now as it has always been; same as strcpy vs. strncpy. The only difference now is that we have increasingly more programmers with very little experience in charge of large software projects.
The people on HN are a big part of those defining the future of Clojure and other languages, so I'm glad to see an article like this raise awareness of something that will bite many as Clojure grows. Lines like these suggest the set-then-unset bit is part of a broader issue:
;; By a weird coincidence this week I wanted to read a file of data
;; coming from a web app, and I was about to use Clojure's reader to
;; do it.
It's disappointing to see that some Clojure practitioners are more interested in downvoting my comment and minimizing that issue rather than educating people about it.
I didn't downvote your comment, but I hold your sentiment in low esteem.
We don't stress over this for the same reason we don't stress over strlen-style errors. They're considered part of the basic education for the language. If someone does do this, they are operating below the expected standard for the language.
There is only so much a language and environment can be expected to do for you. Your understanding of trivial concepts like, "Loading code from untrusted sources is dangerous" is not an unreasonable bar to set. It is only an "issue" if you are fundamentally misinformed about writing software in interpreted environments.
It's bizarre that you can think something is "the expected standard for the language" but also not worth pointing out. How are people supposed to learn it?
There are incompetent programmers that no effort can reach, sure, but there are also junior programmers who are just innocent and haven't yet learned all the implications of what they're doing. I think it's important to teach these people, whether re Ruby or C or Clojure or whatever. That people treat such teaching with disdain makes me think they are misled by their defensiveness. It's not an insult of Clojure to say that (as in any language) there is stuff worth warning people about.
By understanding the halting problem and the implications thereof? This is not Clojure-specific, it's part of any dynamic eval tool.
> It's not an insult of Clojure to say that (as in any language) there is stuff worth warning people about.
This argument might hold some water if the presentation of the information was not couched in a different way. It is clearly not that. And you are clearly more concerned with being right than teaching people, from the words you are choosing to use.
Clojure's reader is designed to provide the types of tasks useful for Lisps; specifically to take a string and convert it to data structures. However, like most Lisps Clojure has some special syntax that does various tasks at read-time like the #= tag that might not be safe to use as a webform processor, but it's critical to the way that Clojure works.
In Clojure 1.5 there are EDN functions that handle a subset of the Clojure syntax and parse strings into data types like numbers, maps and vectors (and others). It's meant to deal with data only and not "reader forms". I wrote a Ring middleware to handle EDN data and recently ported it to use the new 1.5 EDN reader.
Anything that can turn strings into symbols is dangerous code. Symbols can't be garbage collected, and an attacker can conduct a denial of service by forcing you to load unlimited numbers of uniquely-named symbols.
37 comments
[ 2.9 ms ] story [ 87.6 ms ] threadhttp://clojure.github.com/clojure/#clojure.edn
...am i missing something? isn't this the same problem as with evaling json in javascript?
The problem is that there's a hole due to Java constructors being called (even when read-eval is set to false).
In short, don't use read-eval at all if you want safe reading. Use the clojure.edn alternatives to read data safely in those circumstances.
Result:
EDIT: I'm not familiar enough with clojure; is read-line supposed to be safe?
Here I'd say it's a bit different: it's kinda the whole of having read-eval set to true to be able to evaluate code. So when read-eval is set to false and yet rogue code happens to be called there's a serious issue. Basically the issue is that under the hood some Java constructors seems to be called and, if I understand the issue correctly, by knowing how Clojure works you can exploit Clojure / the JVM by crafting precisely what you want to pass to Java constructors (which shouldn't be called in the first place).
Here as I understand it it's not an "eval" exploit per se: it's Java constructors which are passed carefully crafted input.
It isn't supposed to be safe, though it can be tempting to treat it as though it's safe.
- It's similar to the YAML reader, in that Java objects can be created. See "deftype, defrecord, and constructor calls" here: http://clojure.org/reader
- On the positive side for the clojure reader, it isn't widely described as a serialization format and people are less inclined to use it for user input. This is a pretty important difference.
- On the negative side, you can not only instantiate objects, but you can execute code. This is what the example in the article does. You can disable that by setting read-eval.
- Also on the negative side, it lacks something quite as secure as YAML's safe_load. You can tell it not to evaluate code, but there isn't yet a way to disable instantiating objects.
As puredanger pointed out, EDN is a better alternative that also comes with clojure. It's designed for data. Clojure's code format is a superset of EDN. http://clojure.github.com/clojure/clojure.edn-api.html
edn limits what is allowed to a safe subset of what read-line will allow, all of the base clojure types and collection types (maps, lists, sets, vectors) are allowed, Java isn't.
Not quite. It reads raw input and tries to build a syntax tree from it. Unfortunately reader macros in clojure allow all sorts of objects to form part of said syntax tree and arbitrary-ish code to run at parse time to instantiate them. But the main job of the reader isn't to evaluate code.
However, as far as I can tell, the problem here has nothing to do with homoiconicity. The "get string" vulnerability, as you call it, comes out of Clojure's (excellent) JVM support, and not from the Lisp side of its family tree. A "read" is not just a read only because reading Java means creating and compiling arbitrary classes and objects. That, in turn, is largely because Java is not homoiconic.
Personally, I would only use something like read if I were reading a configuration file i.e. something that only someone with privileged access can modify. I would not even consider using read for arbitrary, untrustworthy data; it is just silly in this day and age. It may make the code look prettier, but the fact that arbitrary data structures can be created and the potential to execute arbitrary code is enough to keep me away.
Assuming that by `gets` you mean Ruby's `gets`, this is not quite right. `read` (and `read-string`) are not just generic IO functions. They are parsers for Clojure code. There are plenty of other varied forms of reading input to a string, but `read` does more.
But whether Ruby or C, I take your point that Clojure's read is not just an IO function. It's not really the IO-ness of gets that I'm trying to parallel, but its apparent safety. But perhaps to a Clojure programmer, read is not as surprising as gets because it's more clearly an evaling call. Do you think that'd be clear to a first-year Clojure dev?
It better be. One of the very first topics that a Lisp programmer must know is the implications of separate read/compile/eval phases.
Ah ok, that makes much more sense.
I can't really speak to what is clear or not for a first-year Clojure dev. When I first learned Clojure, I had already dabbled in Lisps and Schemes for a while. The very first time I ran across `read`, probably in Scheme, it was obvious to me that it was special. Did I understand and appreciate the security implications? Probably not.
Hopefully the security issues will be clear to a new Clojure dev since at least the ClojureDocs site steers you away from read and towards EDN. Maybe the official docs should be more stern and clear about it, but they do make a note of the security issue: http://clojure.github.com/clojure/clojure.core-api.html#cloj...
Cultural awareness of read-time evaluation is as frustratingly absent now as it has always been; same as strcpy vs. strncpy. The only difference now is that we have increasingly more programmers with very little experience in charge of large software projects.
We don't stress over this for the same reason we don't stress over strlen-style errors. They're considered part of the basic education for the language. If someone does do this, they are operating below the expected standard for the language.
There is only so much a language and environment can be expected to do for you. Your understanding of trivial concepts like, "Loading code from untrusted sources is dangerous" is not an unreasonable bar to set. It is only an "issue" if you are fundamentally misinformed about writing software in interpreted environments.
There are incompetent programmers that no effort can reach, sure, but there are also junior programmers who are just innocent and haven't yet learned all the implications of what they're doing. I think it's important to teach these people, whether re Ruby or C or Clojure or whatever. That people treat such teaching with disdain makes me think they are misled by their defensiveness. It's not an insult of Clojure to say that (as in any language) there is stuff worth warning people about.
By understanding the halting problem and the implications thereof? This is not Clojure-specific, it's part of any dynamic eval tool.
> It's not an insult of Clojure to say that (as in any language) there is stuff worth warning people about.
This argument might hold some water if the presentation of the information was not couched in a different way. It is clearly not that. And you are clearly more concerned with being right than teaching people, from the words you are choosing to use.
In Clojure 1.5 there are EDN functions that handle a subset of the Clojure syntax and parse strings into data types like numbers, maps and vectors (and others). It's meant to deal with data only and not "reader forms". I wrote a Ring middleware to handle EDN data and recently ported it to use the new 1.5 EDN reader.
https://github.com/fogus/ring-edn/blob/master/src/ring/middl...
It looks almost exactly like the old clojure.core/read-string except it will not execute any dangerous code.