Skip to content
This repository was archived by the owner on Mar 23, 2023. It is now read-only.

Commit f3ebb4a

Browse files
authored
Fix unsafe cast caused by dictNE checking the type of wrong arg (#256)
dictNE was checking v.isInstance(DictType) instead of checking w. This causes a variety of undefined behavior including segfault, invalid mutex state, etc.
1 parent 498b88e commit f3ebb4a

2 files changed

Lines changed: 20 additions & 32 deletions

File tree

runtime/dict.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -705,7 +705,7 @@ func dictLen(f *Frame, o *Object) (*Object, *BaseException) {
705705
}
706706

707707
func dictNE(f *Frame, v, w *Object) (*Object, *BaseException) {
708-
if !v.isInstance(DictType) {
708+
if !w.isInstance(DictType) {
709709
return NotImplemented, nil
710710
}
711711
eq, raised := dictsAreEqual(f, toDictUnsafe(v), toDictUnsafe(w))

runtime/dict_test.go

Lines changed: 19 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -132,34 +132,21 @@ func TestDictDelItemString(t *testing.T) {
132132
}
133133

134134
func TestDictEqNE(t *testing.T) {
135-
fun := newBuiltinFunction("TestDictEqNE", func(f *Frame, args Args, _ KWArgs) (*Object, *BaseException) {
136-
if raised := checkMethodArgs(f, "TestDictEqNE", args, DictType, DictType, BoolType); raised != nil {
137-
return nil, raised
138-
}
139-
d1, d2 := args[0], args[1]
140-
wantEq := toIntUnsafe(args[2]).IsTrue()
141-
if eq, raised := Eq(f, d1, d2); raised != nil {
142-
return nil, raised
143-
} else if !eq.isInstance(BoolType) || toIntUnsafe(eq).IsTrue() != wantEq {
144-
t.Errorf("Eq(%v, %v) = %v, want %v", d1, d2, eq, GetBool(wantEq))
145-
}
146-
if eq, raised := Eq(f, d2, d1); raised != nil {
135+
fun := wrapFuncForTest(func(f *Frame, v, w *Object) (*Object, *BaseException) {
136+
eq, raised := Eq(f, v, w)
137+
if raised != nil {
147138
return nil, raised
148-
} else if !eq.isInstance(BoolType) || toIntUnsafe(eq).IsTrue() != wantEq {
149-
t.Errorf("Eq(%v, %v) = %v, want %v", d2, d1, eq, GetBool(wantEq))
150139
}
151-
if ne, raised := NE(f, d1, d2); raised != nil {
140+
ne, raised := NE(f, v, w)
141+
if raised != nil {
152142
return nil, raised
153-
} else if !ne.isInstance(BoolType) || toIntUnsafe(ne).IsTrue() == wantEq {
154-
t.Errorf("NE(%v, %v) = %v, want %v", d1, d2, ne, GetBool(!wantEq))
155143
}
156-
if ne, raised := NE(f, d2, d1); raised != nil {
144+
valid := GetBool(eq == True.ToObject() && ne == False.ToObject() || eq == False.ToObject() && ne == True.ToObject()).ToObject()
145+
if raised := Assert(f, valid, NewStr("invalid values for __eq__ or __ne__").ToObject()); raised != nil {
157146
return nil, raised
158-
} else if !ne.isInstance(BoolType) || toIntUnsafe(ne).IsTrue() == wantEq {
159-
t.Errorf("NE(%v, %v) = %v, want %v", d2, d1, ne, GetBool(!wantEq))
160147
}
161-
return None, nil
162-
}).ToObject()
148+
return eq, nil
149+
})
163150
f := NewRootFrame()
164151
large1, large2 := NewDict(), NewDict()
165152
largeSize := 100
@@ -177,15 +164,16 @@ func TestDictEqNE(t *testing.T) {
177164
}
178165
o := newObject(ObjectType)
179166
cases := []invokeTestCase{
180-
{args: wrapArgs(NewDict(), NewDict(), true), want: None},
181-
{args: wrapArgs(NewDict(), newTestDict("foo", true), false), want: None},
182-
{args: wrapArgs(newTestDict("foo", "foo"), newTestDict("foo", "foo"), true), want: None},
183-
{args: wrapArgs(newTestDict("foo", true), newTestDict("bar", true), false), want: None},
184-
{args: wrapArgs(newTestDict("foo", true), newTestDict("foo", newObject(ObjectType)), false), want: None},
185-
{args: wrapArgs(newTestDict("foo", true, "bar", false), newTestDict("bar", true), false), want: None},
186-
{args: wrapArgs(newTestDict("foo", o, "bar", o), newTestDict("foo", o, "bar", o), true), want: None},
187-
{args: wrapArgs(newTestDict(2, None, "foo", o), newTestDict("foo", o, 2, None), true), want: None},
188-
{args: wrapArgs(large1, large2, true), want: None},
167+
{args: wrapArgs(NewDict(), NewDict()), want: True.ToObject()},
168+
{args: wrapArgs(NewDict(), newTestDict("foo", true)), want: False.ToObject()},
169+
{args: wrapArgs(newTestDict("foo", "foo"), newTestDict("foo", "foo")), want: True.ToObject()},
170+
{args: wrapArgs(newTestDict("foo", true), newTestDict("bar", true)), want: False.ToObject()},
171+
{args: wrapArgs(newTestDict("foo", true), newTestDict("foo", newObject(ObjectType))), want: False.ToObject()},
172+
{args: wrapArgs(newTestDict("foo", true, "bar", false), newTestDict("bar", true)), want: False.ToObject()},
173+
{args: wrapArgs(newTestDict("foo", o, "bar", o), newTestDict("foo", o, "bar", o)), want: True.ToObject()},
174+
{args: wrapArgs(newTestDict(2, None, "foo", o), newTestDict("foo", o, 2, None)), want: True.ToObject()},
175+
{args: wrapArgs(large1, large2), want: True.ToObject()},
176+
{args: wrapArgs(NewDict(), 123), want: False.ToObject()},
189177
}
190178
for _, cas := range cases {
191179
if err := runInvokeTestCase(fun, &cas); err != "" {

0 commit comments

Comments
 (0)