Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commit14ba32b

Browse files
committed
extmod/vfs_rom: Add bounds checking for all filesystem accesses.
Testing with ROMFS shows that it is relatively easy to end up with acorrupt filesystem on the device -- eg due to the ROMFS deploy processstopping half way through -- which could lead to hard crashes. Notably,there can be boot loops trying to mount a corrupt filesystem, crashes whenimporting modules like `os` that first scan the filesystem for `os.py`, andcrashing when deploying a new ROMFS in certain cases because the old one isremoved while still mounted.The main problem is that `mp_decode_uint()` has an loop that keeps going aslong as it reads 0xff byte values, which can happen in the case of erasedand unwritten flash.This commit adds full bounds checking in the new `mp_decode_uint_checked()`function, and that makes all ROMFS filesystem accesses robust.Signed-off-by: Damien George <damien@micropython.org>
1 parente3101ce commit14ba32b

File tree

2 files changed

+163
-24
lines changed

2 files changed

+163
-24
lines changed

‎extmod/vfs_rom.c

Lines changed: 90 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -91,6 +91,7 @@
9191
#defineROMFS_RECORD_KIND_DATA_POINTER (3)
9292
#defineROMFS_RECORD_KIND_DIRECTORY (4)
9393
#defineROMFS_RECORD_KIND_FILE (5)
94+
#defineROMFS_RECORD_KIND_FILESYSTEM (0x14a6b1)
9495

9596
typedefmp_uint_trecord_kind_t;
9697

@@ -101,34 +102,72 @@ struct _mp_obj_vfs_rom_t {
101102
constuint8_t*filesystem_end;
102103
};
103104

104-
staticrecord_kind_textract_record(constuint8_t**fs,constuint8_t**fs_next) {
105-
record_kind_trecord_kind=mp_decode_uint(fs);
106-
mp_uint_trecord_len=mp_decode_uint(fs);
105+
// Returns 0 for success, -1 for failure.
106+
staticintmp_decode_uint_checked(constuint8_t**ptr,constuint8_t*ptr_max,mp_uint_t*value_out) {
107+
mp_uint_tunum=0;
108+
byteval;
109+
constuint8_t*p=*ptr;
110+
do {
111+
if (p >=ptr_max) {
112+
return-1;
113+
}
114+
val=*p++;
115+
unum= (unum <<7) | (val&0x7f);
116+
}while ((val&0x80)!=0);
117+
*ptr=p;
118+
*value_out=unum;
119+
return0;
120+
}
121+
122+
staticrecord_kind_textract_record(constuint8_t**fs,constuint8_t**fs_next,constuint8_t*fs_max) {
123+
mp_uint_trecord_kind;
124+
if (mp_decode_uint_checked(fs,fs_max,&record_kind)!=0) {
125+
returnROMFS_RECORD_KIND_UNUSED;
126+
}
127+
mp_uint_trecord_len;
128+
if (mp_decode_uint_checked(fs,fs_max,&record_len)!=0) {
129+
returnROMFS_RECORD_KIND_UNUSED;
130+
}
107131
*fs_next=*fs+record_len;
108132
returnrecord_kind;
109133
}
110134

111-
staticvoidextract_data(mp_obj_vfs_rom_t*self,constuint8_t*fs,constuint8_t*fs_top,size_t*size_out,constuint8_t**data_out) {
112-
*size_out=0;
113-
*data_out=NULL;
135+
// Returns 0 for success, a negative integer for failure.
136+
staticintextract_data(mp_obj_vfs_rom_t*self,constuint8_t*fs,constuint8_t*fs_top,size_t*size_out,constuint8_t**data_out) {
114137
while (fs<fs_top) {
115138
constuint8_t*fs_next;
116-
record_kind_trecord_kind=extract_record(&fs,&fs_next);
117-
if (record_kind==ROMFS_RECORD_KIND_DATA_VERBATIM) {
118-
// Verbatim data.
119-
*size_out=fs_next-fs;
120-
*data_out=fs;
139+
record_kind_trecord_kind=extract_record(&fs,&fs_next,fs_top);
140+
if (record_kind==ROMFS_RECORD_KIND_UNUSED) {
141+
// Corrupt filesystem.
121142
break;
143+
}elseif (record_kind==ROMFS_RECORD_KIND_DATA_VERBATIM) {
144+
// Verbatim data.
145+
if (size_out!=NULL) {
146+
*size_out=fs_next-fs;
147+
*data_out=fs;
148+
}
149+
return0;
122150
}elseif (record_kind==ROMFS_RECORD_KIND_DATA_POINTER) {
123151
// Pointer to data.
124-
*size_out=mp_decode_uint(&fs);
125-
*data_out=self->filesystem+mp_decode_uint(&fs);
126-
break;
152+
mp_uint_tsize;
153+
if (mp_decode_uint_checked(&fs,fs_next,&size)!=0) {
154+
break;
155+
}
156+
mp_uint_toffset;
157+
if (mp_decode_uint_checked(&fs,fs_next,&offset)!=0) {
158+
break;
159+
}
160+
if (size_out!=NULL) {
161+
*size_out=size;
162+
*data_out=self->filesystem+offset;
163+
}
164+
return0;
127165
}else {
128166
// Skip this record.
129167
fs=fs_next;
130168
}
131169
}
170+
return-MP_EIO;
132171
}
133172

134173
// Searches for `path` in the filesystem.
@@ -144,10 +183,17 @@ mp_import_stat_t mp_vfs_rom_search_filesystem(mp_obj_vfs_rom_t *self, const char
144183
}
145184
while (path_len>0&&fs<fs_top) {
146185
constuint8_t*fs_next;
147-
record_kind_trecord_kind=extract_record(&fs,&fs_next);
148-
if (record_kind==ROMFS_RECORD_KIND_DIRECTORY||record_kind==ROMFS_RECORD_KIND_FILE) {
186+
record_kind_trecord_kind=extract_record(&fs,&fs_next,fs_top);
187+
if (record_kind==ROMFS_RECORD_KIND_UNUSED) {
188+
// Corrupt filesystem.
189+
returnMP_IMPORT_STAT_NO_EXIST;
190+
}elseif (record_kind==ROMFS_RECORD_KIND_DIRECTORY||record_kind==ROMFS_RECORD_KIND_FILE) {
149191
// A directory or file record.
150-
mp_uint_tname_len=mp_decode_uint(&fs);
192+
mp_uint_tname_len;
193+
if (mp_decode_uint_checked(&fs,fs_next,&name_len)!=0) {
194+
// Corrupt filesystem.
195+
returnMP_IMPORT_STAT_NO_EXIST;
196+
}
151197
if ((name_len==path_len
152198
|| (name_len<path_len&&path[name_len]=='/'))
153199
&&memcmp(path,fs,name_len)==0) {
@@ -167,8 +213,9 @@ mp_import_stat_t mp_vfs_rom_search_filesystem(mp_obj_vfs_rom_t *self, const char
167213
if (path_len!=0) {
168214
returnMP_IMPORT_STAT_NO_EXIST;
169215
}
170-
if (size_out!=NULL) {
171-
extract_data(self,fs,fs_top,size_out,data_out);
216+
if (extract_data(self,fs,fs_top,size_out,data_out)!=0) {
217+
// Corrupt filesystem.
218+
returnMP_IMPORT_STAT_NO_EXIST;
172219
}
173220
returnMP_IMPORT_STAT_FILE;
174221
}
@@ -214,7 +261,15 @@ static mp_obj_t vfs_rom_make_new(const mp_obj_type_t *type, size_t n_args, size_
214261
}
215262

216263
// The ROMFS is a record itself, so enter into it and compute its limit.
217-
extract_record(&self->filesystem,&self->filesystem_end);
264+
record_kind_trecord_kind=extract_record(&self->filesystem,&self->filesystem_end,self->filesystem+bufinfo.len);
265+
if (record_kind!=ROMFS_RECORD_KIND_FILESYSTEM) {
266+
mp_raise_OSError(MP_ENODEV);
267+
}
268+
269+
// Check the filesystem is within the limits of the input buffer.
270+
if (self->filesystem_end> (constuint8_t*)bufinfo.buf+bufinfo.len) {
271+
mp_raise_OSError(MP_ENODEV);
272+
}
218273

219274
returnMP_OBJ_FROM_PTR(self);
220275
}
@@ -259,13 +314,21 @@ static mp_obj_t vfs_rom_ilistdir_it_iternext(mp_obj_t self_in) {
259314

260315
while (self->index<self->index_top) {
261316
constuint8_t*index_next;
262-
record_kind_trecord_kind=extract_record(&self->index,&index_next);
317+
record_kind_trecord_kind=extract_record(&self->index,&index_next,self->index_top);
263318
uint32_ttype;
264319
mp_uint_tname_len;
265320
size_tdata_len;
266-
if (record_kind==ROMFS_RECORD_KIND_DIRECTORY||record_kind==ROMFS_RECORD_KIND_FILE) {
321+
if (record_kind==ROMFS_RECORD_KIND_UNUSED) {
322+
// Corrupt filesystem.
323+
self->index=self->index_top;
324+
break;
325+
}elseif (record_kind==ROMFS_RECORD_KIND_DIRECTORY||record_kind==ROMFS_RECORD_KIND_FILE) {
267326
// A directory or file record.
268-
name_len=mp_decode_uint(&self->index);
327+
if (mp_decode_uint_checked(&self->index,index_next,&name_len)!=0) {
328+
// Corrupt filesystem.
329+
self->index=self->index_top;
330+
break;
331+
}
269332
if (record_kind==ROMFS_RECORD_KIND_DIRECTORY) {
270333
// A directory.
271334
type=MP_S_IFDIR;
@@ -274,7 +337,10 @@ static mp_obj_t vfs_rom_ilistdir_it_iternext(mp_obj_t self_in) {
274337
// A file.
275338
type=MP_S_IFREG;
276339
constuint8_t*data_value;
277-
extract_data(self->vfs_rom,self->index+name_len,index_next,&data_len,&data_value);
340+
if (extract_data(self->vfs_rom,self->index+name_len,index_next,&data_len,&data_value)!=0) {
341+
// Corrupt filesystem.
342+
break;
343+
}
278344
}
279345
}else {
280346
// Skip this record.

‎tests/extmod/vfs_rom.py

Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -223,6 +223,79 @@ def test_unknown_record(self):
223223
self.assertEqual(f.read(),b"contents")
224224

225225

226+
classTestCorrupt(unittest.TestCase):
227+
deftest_corrupt_filesystem(self):
228+
# Make the filesystem length bigger than the buffer.
229+
romfs=bytearray(make_romfs(()))
230+
romfs[3]=0x01
231+
withself.assertRaises(OSError):
232+
vfs.VfsRom(romfs)
233+
234+
# Corrupt the filesystem length.
235+
romfs=bytearray(make_romfs(()))
236+
romfs[3]=0xFF
237+
withself.assertRaises(OSError):
238+
vfs.VfsRom(romfs)
239+
240+
# Corrupt the contents of the filesystem.
241+
romfs=bytearray(make_romfs(()))
242+
romfs[3]=0x01
243+
romfs.extend(b"\xff\xff")
244+
fs=vfs.VfsRom(romfs)
245+
withself.assertRaises(OSError):
246+
fs.stat("a")
247+
self.assertEqual(list(fs.ilistdir("")), [])
248+
249+
deftest_corrupt_file_entry(self):
250+
romfs=make_romfs((("file",b"data"),))
251+
252+
# Corrupt the length of filename.
253+
romfs_corrupt=bytearray(romfs)
254+
romfs_corrupt[7:]=b"\xff"* (len(romfs)-7)
255+
fs=vfs.VfsRom(romfs_corrupt)
256+
withself.assertRaises(OSError):
257+
fs.stat("file")
258+
self.assertEqual(list(fs.ilistdir("")), [])
259+
260+
# Erase the data record (change it to a padding record).
261+
romfs_corrupt=bytearray(romfs)
262+
romfs_corrupt[12]=VfsRomWriter.ROMFS_RECORD_KIND_PADDING
263+
fs=vfs.VfsRom(romfs_corrupt)
264+
withself.assertRaises(OSError):
265+
fs.stat("file")
266+
self.assertEqual(list(fs.ilistdir("")), [])
267+
268+
# Corrupt the header of the data record.
269+
romfs_corrupt=bytearray(romfs)
270+
romfs_corrupt[12:]=b"\xff"* (len(romfs)-12)
271+
fs=vfs.VfsRom(romfs_corrupt)
272+
withself.assertRaises(OSError):
273+
fs.stat("file")
274+
275+
# Corrupt the interior of the data record.
276+
romfs_corrupt=bytearray(romfs)
277+
romfs_corrupt[13:]=b"\xff"* (len(romfs)-13)
278+
fs=vfs.VfsRom(romfs_corrupt)
279+
withself.assertRaises(OSError):
280+
fs.stat("file")
281+
282+
# Change the data record to an indirect pointer and corrupt the length.
283+
romfs_corrupt=bytearray(romfs)
284+
romfs_corrupt[12]=VfsRomWriter.ROMFS_RECORD_KIND_DATA_POINTER
285+
romfs_corrupt[14:18]=b"\xff\xff\xff\xff"
286+
fs=vfs.VfsRom(romfs_corrupt)
287+
withself.assertRaises(OSError):
288+
fs.stat("file")
289+
290+
# Change the data record to an indirect pointer and corrupt the offset.
291+
romfs_corrupt=bytearray(romfs)
292+
romfs_corrupt[12]=VfsRomWriter.ROMFS_RECORD_KIND_DATA_POINTER
293+
romfs_corrupt[14:18]=b"\x00\xff\xff\xff"
294+
fs=vfs.VfsRom(romfs_corrupt)
295+
withself.assertRaises(OSError):
296+
fs.stat("file")
297+
298+
226299
classTestStandalone(TestBase):
227300
deftest_constructor(self):
228301
self.assertIsInstance(vfs.VfsRom(self.romfs),vfs.VfsRom)

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp