Opened 9 years ago
Closed 9 years ago
#1216 closed defect (fixed)
string-ref specialization elides range check
Reported by: | Moritz Heidkamp | Owned by: | |
---|---|---|---|
Priority: | critical | Milestone: | 4.11.0 |
Component: | scrutinizer | Version: | 4.10.x |
Keywords: | Cc: | ||
Estimated difficulty: |
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.
Change History (5)
comment:1 Changed 9 years ago by
comment:2 Changed 9 years ago by
Priority: | major → critical |
---|
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 results in unsafe specializations which indeed 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.
comment:3 Changed 9 years ago by
Milestone: | someday → 4.11.0 |
---|
comment:5 Changed 9 years ago by
Resolution: | → fixed |
---|---|
Status: | new → closed |
Fixed by 3a15b29 and 55e9d1e
I would guess that similar issues exist for other specializations, too.