Summary

string-ref specialization elides range check

Metadata

Description

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.

Changes and comments

[2015-08-26 22:11:39 UTC] syn wrote:

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

[2015-08-27 08:36:26 UTC] sjamaan changed priority from major to critical

[2015-08-27 08:36:26 UTC] sjamaan wrote:

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.

[2015-08-27 08:36:26 UTC] sjamaan wrote:

1440657446051912

[2015-08-27 08:40:22 UTC] sjamaan changed milestone from someday to 4.11.0

[2015-08-27 08:48:37 UTC] sjamaan wrote:

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

[2015-09-10 20:56:56 UTC] sjamaan changed status from new to closed

[2015-09-10 20:56:56 UTC] sjamaan set resolution to fixed

[2015-09-10 20:56:56 UTC] sjamaan wrote:

Fixed by 3a15b29 and 55e9d1e