Skip to content

Commit ae4cbdb

Browse files
Protect Box.unbox from dereferencing null pointer (#16514)
Unboxing a null pointer leads to invalid memory access when the target type is not nilable. We can avoid this by raising an explicit exception. This makes `Box.unbox` a little bit safer. Of course, we can still not protect against any other invalid pointer. But null pointers are easily identifiable as invalid. This is low-hanging fruit for a bit more implicit memory safety.
1 parent 37cdfa7 commit ae4cbdb

File tree

2 files changed

+40
-1
lines changed

2 files changed

+40
-1
lines changed

spec/std/box_spec.cr

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,32 @@ describe "Box" do
5757
Box(Nil).unbox(box).should be_nil
5858
end
5959

60+
describe "unboxing a null pointer" do
61+
it "returns nil if nilable" do
62+
Box(Nil).unbox(Pointer(Void).null).should be_nil
63+
Box(String?).unbox(Pointer(Void).null).should be_nil
64+
Box(UInt8*).unbox(Pointer(Void).null).should eq Pointer(UInt8).null
65+
end
66+
67+
it "raises if non-nilable" do
68+
expect_raises(NilAssertionError, "Unboxing null pointer") do
69+
Box(String).unbox(Pointer(Void).null)
70+
end
71+
expect_raises(NilAssertionError, "Unboxing null pointer") do
72+
Box(Int32).unbox(Pointer(Void).null)
73+
end
74+
end
75+
76+
it "raises for mixed union" do
77+
expect_raises(NilAssertionError, "Unboxing null pointer in mixed union") do
78+
Box(Int32 | String?).unbox(Pointer(Void).null)
79+
end
80+
expect_raises(NilAssertionError, "Unboxing null pointer in mixed union") do
81+
Box(Int32?).unbox(Pointer(Void).null)
82+
end
83+
end
84+
end
85+
6086
it "boxing nil in a reference-like union returns a null pointer (#11839)" do
6187
box = Box.box(nil.as(String?))
6288
box.address.should eq(0)

src/box.cr

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,13 +39,26 @@ class Box(T)
3939
# Unboxes a `Void*` into an object of type `T`. Note that for this you must
4040
# specify T: `Box(T).unbox(data)`.
4141
#
42+
# Raises `NilAssertionError` if *pointer* is null and `T` is not nilable.
43+
#
4244
# WARNING: It is undefined behavior to box an object in one type and unbox it
4345
# via a different type; in particular, when boxing a `T` and unboxing it as a
4446
# `T?`, or vice-versa.
4547
def self.unbox(pointer : Void*) : T
46-
{% if T < Pointer || T.union_types.all? { |t| t == Nil || t < Reference } %}
48+
{% if T < Pointer %}
49+
pointer.as(T)
50+
{% elsif T.union_types.all? { |t| t == Nil || t < Reference } %}
51+
{% unless T.union_types.any? { |t| t == Nil } %}
52+
if pointer.null?
53+
raise NilAssertionError.new("Unboxing null pointer")
54+
end
55+
{% end %}
4756
pointer.as(T)
4857
{% else %}
58+
if pointer.null?
59+
raise NilAssertionError.new("Unboxing null pointer in mixed union")
60+
end
61+
4962
pointer.as(self).object
5063
{% end %}
5164
end

0 commit comments

Comments
 (0)