Skip to content

Commit 6db91df

Browse files
BottleRocketeerdpgeorge
authored andcommitted
extmod/modbtree: Add checks for already-closed database.
Fixes use-after-free when accessing the database after it is closed with `btree_close`. `btree_close` always succeeds when called with an already-closed database. The new test checks that operations that access the underlying database (get, set, flush, seq) fail with a `ValueError` when the btree is already closed. It also checks that closing and printing the btree succeed when the btree is already closed. Fixes issue micropython#12543. Signed-off-by: Michael Vornovitsky <[email protected]>
1 parent 8159dcc commit 6db91df

File tree

3 files changed

+65
-1
lines changed

3 files changed

+65
-1
lines changed

extmod/modbtree.c

+21-1
Original file line numberDiff line numberDiff line change
@@ -89,6 +89,12 @@ void __dbpanic(DB *db) {
8989
mp_printf(&mp_plat_print, "__dbpanic(%p)\n", db);
9090
}
9191

92+
static void check_btree_is_open(mp_obj_btree_t *self) {
93+
if (!self->db) {
94+
mp_raise_ValueError(MP_ERROR_TEXT("database closed"));
95+
}
96+
}
97+
9298
static mp_obj_btree_t *btree_new(DB *db, mp_obj_t stream) {
9399
mp_obj_btree_t *o = mp_obj_malloc(mp_obj_btree_t, (mp_obj_type_t *)&btree_type);
94100
o->stream = stream;
@@ -114,19 +120,28 @@ static void btree_print(const mp_print_t *print, mp_obj_t self_in, mp_print_kind
114120

115121
static mp_obj_t btree_flush(mp_obj_t self_in) {
116122
mp_obj_btree_t *self = MP_OBJ_TO_PTR(self_in);
123+
check_btree_is_open(self);
117124
return MP_OBJ_NEW_SMALL_INT(__bt_sync(self->db, 0));
118125
}
119126
static MP_DEFINE_CONST_FUN_OBJ_1(btree_flush_obj, btree_flush);
120127

121128
static mp_obj_t btree_close(mp_obj_t self_in) {
122129
mp_obj_btree_t *self = MP_OBJ_TO_PTR(self_in);
123-
return MP_OBJ_NEW_SMALL_INT(__bt_close(self->db));
130+
int res;
131+
if (self->db) {
132+
res = __bt_close(self->db);
133+
self->db = NULL;
134+
} else {
135+
res = RET_SUCCESS; // Closing an already-closed DB always succeeds.
136+
}
137+
return MP_OBJ_NEW_SMALL_INT(res);
124138
}
125139
static MP_DEFINE_CONST_FUN_OBJ_1(btree_close_obj, btree_close);
126140

127141
static mp_obj_t btree_put(size_t n_args, const mp_obj_t *args) {
128142
(void)n_args;
129143
mp_obj_btree_t *self = MP_OBJ_TO_PTR(args[0]);
144+
check_btree_is_open(self);
130145
DBT key, val;
131146
buf_to_dbt(args[1], &key);
132147
buf_to_dbt(args[2], &val);
@@ -136,6 +151,7 @@ static MP_DEFINE_CONST_FUN_OBJ_VAR_BETWEEN(btree_put_obj, 3, 4, btree_put);
136151

137152
static mp_obj_t btree_get(size_t n_args, const mp_obj_t *args) {
138153
mp_obj_btree_t *self = MP_OBJ_TO_PTR(args[0]);
154+
check_btree_is_open(self);
139155
DBT key, val;
140156
buf_to_dbt(args[1], &key);
141157
int res = __bt_get(self->db, &key, &val, 0);
@@ -153,6 +169,7 @@ static MP_DEFINE_CONST_FUN_OBJ_VAR_BETWEEN(btree_get_obj, 2, 3, btree_get);
153169

154170
static mp_obj_t btree_seq(size_t n_args, const mp_obj_t *args) {
155171
mp_obj_btree_t *self = MP_OBJ_TO_PTR(args[0]);
172+
check_btree_is_open(self);
156173
int flags = MP_OBJ_SMALL_INT_VALUE(args[1]);
157174
DBT key, val;
158175
if (n_args > 2) {
@@ -225,6 +242,7 @@ static mp_obj_t btree_getiter(mp_obj_t self_in, mp_obj_iter_buf_t *iter_buf) {
225242

226243
static mp_obj_t btree_iternext(mp_obj_t self_in) {
227244
mp_obj_btree_t *self = MP_OBJ_TO_PTR(self_in);
245+
check_btree_is_open(self);
228246
DBT key, val;
229247
int res;
230248
bool desc = self->flags & FLAG_DESC;
@@ -281,6 +299,7 @@ static mp_obj_t btree_iternext(mp_obj_t self_in) {
281299

282300
static mp_obj_t btree_subscr(mp_obj_t self_in, mp_obj_t index, mp_obj_t value) {
283301
mp_obj_btree_t *self = MP_OBJ_TO_PTR(self_in);
302+
check_btree_is_open(self);
284303
if (value == MP_OBJ_NULL) {
285304
// delete
286305
DBT key;
@@ -314,6 +333,7 @@ static mp_obj_t btree_subscr(mp_obj_t self_in, mp_obj_t index, mp_obj_t value) {
314333

315334
static mp_obj_t btree_binary_op(mp_binary_op_t op, mp_obj_t lhs_in, mp_obj_t rhs_in) {
316335
mp_obj_btree_t *self = MP_OBJ_TO_PTR(lhs_in);
336+
check_btree_is_open(self);
317337
switch (op) {
318338
case MP_BINARY_OP_CONTAINS: {
319339
DBT key, val;

tests/extmod/btree_closed.py

+39
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
try:
2+
import btree
3+
import io
4+
except ImportError:
5+
print("SKIP")
6+
raise SystemExit
7+
8+
f = io.BytesIO()
9+
db = btree.open(f)
10+
11+
db[b"foo"] = b"42"
12+
13+
db.close()
14+
15+
# Accessing an already-closed database should fail.
16+
try:
17+
print(db[b"foo"])
18+
except ValueError:
19+
print("ValueError")
20+
21+
try:
22+
db[b"bar"] = b"43"
23+
except ValueError:
24+
print("ValueError")
25+
26+
try:
27+
db.flush()
28+
except ValueError:
29+
print("ValueError")
30+
31+
try:
32+
for k, v in db.items():
33+
pass
34+
except ValueError:
35+
print("ValueError")
36+
37+
# Closing and printing an already-closed database should not fail.
38+
db.close()
39+
print(db)

tests/extmod/btree_closed.py.exp

+5
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
ValueError
2+
ValueError
3+
ValueError
4+
ValueError
5+
<btree 0>

0 commit comments

Comments
 (0)