Re: Is there any security risk related to the use of the reader?

[Christopher Lemmer Webber <cwebber@dustycloud.org>]

Luckily I kept notes on this... the answer is not very if you're
snarfing stuff over the wire!

: <wingo> paroneayea: relatedly,
: 	http://git.savannah.gnu.org/cgit/guile.git/commit/?id=e68dd5c601ef7975507d4118bcc2ad334b0450b2
: <wingo> i suppose that doc update doesn't touch security at all tho...  [16:54]
: <paroneayea> wingo: ah nice
: <paroneayea> wingo: yeah... srfi-10 seems to be one way where (read) can get
: 	     scary						        [16:55]
: <paroneayea> wingo: really makes me wish I could specify a scope where I
: 	     *know* how read behaves
: <wingo> yeah i know what you mean				        [16:56]
: <paroneayea> maybe (scoped-read) (scoped-write)
: <paroneayea> or even a module that provides (read) and (write), splatting over
: 	     the default ones unless you namespace it :)	        [16:57]
: <wingo> nah, i think a port option or something is what you want
: <paroneayea> a port option would be fine
: <dsmith-work> There is also read-hash-extend (I think srfi-10 uses that)
: <paroneayea> wingo: I saw that there was some mention of being able to control
: 	     how read works in the port... but it looked like the *data* read
: 	     by read was able to change read behavior		        [16:58]
: <paroneayea> which is actually even more worrying!
: <wingo> how could the data change the behavior?  only via #.
: <wingo> which is off by default
: <paroneayea> ah
: <paroneayea> okay whew
: <wingo> no one in their right mind turns that one on :)
: <wingo> there's also #!r6rs i think...				        [16:59]
: <paroneayea> wingo: I think this paragraph is not clear to me:
: <wingo> that changes some parameters, but they are static parameters, and
: 	maybe only in certain situations
: <wingo> anyway not like a danger or anything
: <paroneayea> https://dpaste.de/Pdor
: <paroneayea> wingo: nothing in that section mentions how # gets turned on to
: 	     enable that					        [17:00]
: <paroneayea> it sounds like *any* port that runs across #!fold-case will start
: 	     doing that to me..
: <paroneayea> but maybe I'm just not understandgin while reading	        [17:01]
: <wingo> no, that's correct
: <wingo> maybe it's a problem? dunno
: <paroneayea> it doesn't seem like those are dangerous options
: <wingo> when #!fold-case is in a file that's certainly what you want
: <wingo> if an attacker can inject #!fold-case in some thing...	        [17:02]
: <wingo> it's an interesting idea, i hadn't thought about that
: <wingo> mark_weaver will like that one :)
: <paroneayea> well, the question is whether (read) is "safe" when you get
: 	     arbitrary incoming data				        [17:03]
: <wingo> we probably need to have a port option to disable these things i guess
: <paroneayea> in the case I'm in, a signature is checked before it ever hits
: 	     (read), but could read/write be as safe as json?
: <wingo> paroneayea: what does "safe" mean in this context?  threat model
: 	missing :)
: <paroneayea> wingo: think "reading data off a REST style API"
: <wingo> and?							        [17:04]
: <davexunit> arbitrary code execution
: <paroneayea> yes
: <paroneayea> eg, you're probably at no risk parsing that data through json
: <wingo> so
: <davexunit> which I don't think is possible
: <paroneayea> every rest api in the world does that
: <wingo> arbitrary code execution is only possible via read-eval (the #. thing)
: <wingo> that should be off globally.
: <paroneayea> davexunit: what about if srfi-10 is on? :)
: <wingo> you can get some code execution via srfi-10 or read-hash-extend
: <wingo> so you have to know whether the extensions that are active in your
: 	program are safe against your threat model		        [17:05]
: <wingo> for example
: <wingo> a syntax extension which turns #'foo into (syntax foo)
: <wingo> there's no danger there.
: <paroneayea> sure
: <paroneayea> though say I decided "to hell with json, my REST API is going to
: 	     use sexps!  Wheee!"				        [17:06]
: <paroneayea> and then I import a module that itself imports srfi-10
: <paroneayea> and I had no idea
: <paroneayea> and bam, code execution seems possible
: <wingo> yeah, not full #. code execution but yes		        [17:07]
: <wingo> but that same argument applies if a module enables #.
: <paroneayea> wingo: right
: <wingo> so yeah, it's a property of your whole program
: <paroneayea> wingo: basically I'm saying, I don't want either of those to
: 	     happen
: <paroneayea> because I don't know much about my libraries, necessarily
: <wingo> what code can be run by `read'				        [17:08]
: <wingo> yeah i know what you mean.  i solve this by being an asshole and never
: 	using other people's code :/  not a great solution
: <dsmith-work> paroneayea: You could purge srfi-10 from systems you control?
: <dsmith-work> If some module needs it, too bad.			        [17:09]
: <wingo> if you want to control the property globally, yeah like dsmith-work
: 	says we can add global kill switches
: <davexunit> how about a dumb-read procedure?
: <davexunit> no reader macros allowed :)
: <wingo> we could also add per-port kill switches but that's less convenient in
: 	some ways
: <wingo> heh, at that point just write your own `read' :)
: <wingo> there's also some memory dangers			        [17:10]
: <wingo> e.g. reading 10e10000
: <wingo> rather #e10e1000
: <wingo> anyway
: <davexunit> I don't know read is implemented, so I don't know how feasible it
: 	    would be to say "don't use any reader macros for this read call"
: <dsmith-work> What about malformed sexpr?
: <dsmith-work> Like reading "(((foo" 				        [17:11]
: <wingo> read is implemented in c right now.  for options it's better to attach
: 	them to the port.
: <paroneayea> wingo: wow #e10e1000 is interesting
: <paroneayea> I would have never thought of that.		        [17:12]
: <wingo> irritating isn't it :)
: <paroneayea> so, this conversation is making me feel like: don't use read for
: 	     any data that comes over the wire that you don't really trust
: <paroneayea> or use a safer one you write yourself :)		        [17:13]
: <paroneayea> so, it's still safe with the signed sessions, because an attacker
: 	     would need your key to get you to do something evil with read
: 								        [17:14]
: <paroneayea> but #e10e10000, srfi-10, and #. are all worrying possible attacks
: 	     against using vanilla read for much data heading over the wire
: 	     that you don't control				        [17:15]
: <wingo> i think there is another attack, which is ((((((((((((((((((((((((((
: 								        [17:16]
: <wingo> keep on going
: <wingo> the c stack keeps recursing, eventually segfault i think
: <paroneayea> wingo: interesting					        [17:17]
: <paroneayea> wingq: I guess the json parser I'm using is also semi-vulnerable
: 	     to that, but it wouldn't be in C
: <paroneayea> so probably there's more safety
: <paroneayea> wouldn't segfault, anyway
: * wingo currently trying the ((((( attack
: <wingo> do you have a limit on your message size?		        [17:18]
: <paroneayea> wingo: for this particular case (sessions), the limit is 4kb, but
: 	     I'm not worried about that.  Probably for the json-ld stuff
: 	     coming over the wire... I haven't set any limit :)	        [17:19]
: <wingo> lol
: <wingo> scheme@(guile-user)> (call-with-input-string (make-string 200000 #\()
: 	read)
: <wingo> Segmentation fault
: <paroneayea> whee :)
: <wingo> erryting turribul					        [17:20]
: * paroneayea saves this conversation in eir notes.org file :)
: <wingo> this is "just" a bug					        [17:21]
: <dsmith-work> IS there a limit to cookie size?
: <dsmith-work> paroneayea: or whatever you are planning on read'ing
: <paroneayea> dsmith-work: yes, cookies are limited to 4kb	        [17:22]
: <paroneayea> according to the HTTP spec
: <paroneayea> well, whether guile does that
: <paroneayea> I haven't checked yet
: <paroneayea> (or rather, headers are)
: <dsmith-work> scheme@(guile-user)> (call-with-input-string (make-string 4096
: 	      #\() read)
: <dsmith-work> ERROR: In procedure read:
: <dsmith-work> ERROR: In procedure scm_i_lreadparen: #<unknown port>:1:4097:
: 	      end of file
: <paroneayea> or if not according to the http spec, according to browser
: 	     conventions :)
: <paroneayea>
: 	     http://stackoverflow.com/questions/640938/what-is-the-maximum-size-of-a-web-browsers-cookies-key
: <dsmith-work> So in your case, at least you won't segfault.
: <paroneayea> not sure if this is from any spec, or just convention
: <paroneayea> dsmith-work: in my particular case, for sessions, it's still not
: 	     risky because again, someone would need to forge the key...
: 								        [17:24]
: <paroneayea> but yeah
: <paroneayea> I'm still interested in understanding in general how "safe" read
: 	     / write are in various scenarios


Amirouche Boubekki writes:

> I have procedures like that in my program:
>
> (define-public (scm->string scm)
>    (call-with-output-string
>      (lambda (port)
>        (write scm port))))
>
> (define-public (string->scm string)
>    (call-with-input-string string read))
>
> Is it safe to pass to this procedures input from third parties?
>
> TIA!