string-ref specialization elides range check

[Moritz Heidkamp]

The types.db entry for string-ref currently looks like this:

(string-ref (#(procedure #:clean #:enforce) string-ref (string fixnum) char)
	    ((string fixnum) (##core#inline "C_subchar" #(1) #(2))))

However, unlike C_subchar, the unspecialized version of string-ref (which is really C_i_string_ref) doesn't just check its argument type but also whether the fixnum argument is within range of the string argument. Thus, the specialization may result in buffer overruns, posing a potential vulnerability.

[Moritz Heidkamp]

I would guess that similar issues exist for other specializations, too.

[sjamaan]

Changing it to C_i_string_ref would still be beneficial due to inlining. Note that there are also two rewrites in c-platform.scm; an unsafe one that uses C_subchar and a safe one that uses C_i_string_ref.

I think the real problem here is that there are two kinds of unsafe: the kind that doesn't check its argument types (which will result in crashes when passed the wrong type; but in the scrutinizer that means it wouldn't be unsafe because the types are checked elsewhere) and the kind that doesn't check anything. The latter should never be used in specializations, because they will result in unsafe code, which in turn may result in security nightmares.

I'm raising this ticket to "critical" because there may be some true landmines waiting to go off in Scheme code that's otherwise safe. Rewrites like this change the semantics of the code in such a way that you can't reason about its safety anymore.

[sjamaan]

About those rewrites: This may be another instance of #1122

[sjamaan]

Fixed by 3a15b29 and 55e9d1e