Summary
I spotted an off-by-one buffer overflow in RIOT source code at:
https://github.com/RIOT-OS/RIOT/blob/master/pkg/lwext4/fs/lwext4_fs.c#L115
I also spotted the lack of explicit NUL-termination after strncpy()
in RIOT source code at:
https://github.com/RIOT-OS/RIOT/blob/master/pkg/lwext4/fs/lwext4_fs.c#L464
Details
If an attacker is able to make mount_point
at least CONFIG_EXT4_MAX_MP_NAME
bytes large, strcat()
would write a NUL byte past the end of the mp.name
buffer, thus potentially overwriting the next member of the ext4_mountpoint
struct described here.
Please refer to the marked line below:
static int prepare(lwext4_desc_t *fs, const char *mount_point)
{
mtd_dev_t *dev = fs->dev;
struct ext4_blockdev_iface *iface = &fs->iface;
memset(&fs->mp, 0, sizeof(fs->mp));
memset(&fs->bdev, 0, sizeof(fs->bdev));
memset(&fs->iface, 0, sizeof(fs->iface));
strncpy(fs->mp.name, mount_point, CONFIG_EXT4_MAX_MP_NAME);
strcat(fs->mp.name, "/"); // VULN: off-by-one buffer overflow
int res = mtd_init(dev);
if (res) {
return res;
}
...
Since sizeof(dirent->name)
is 255 and sizeof(entry->d_name)
is only 32, if an attacker can control the input to strncpy()
they would be able to create a non NUL-terminated string in entry->d_name
. When such corrupted string is used in other parts of the code, it may cause information leakage or memory corruption.
Please refer to the marked line below:
static int _readdir(vfs_DIR *dirp, vfs_dirent_t *entry)
{
ext4_dir *dir = _get_ext4_dir(dirp);
const ext4_direntry *dirent = ext4_dir_entry_next(dir);
if (dirent == NULL) {
return 0;
}
strncpy(entry->d_name, (char *)dirent->name, sizeof(entry->d_name)); // VULN
return 1;
}
Impact
In my understanding, the impact of these vulnerabilities in this specific case is quite limited. However, all bugs that have the potential to cause memory corruption should be taken seriously and fixed as security bugs.
Summary
I spotted an off-by-one buffer overflow in RIOT source code at:
https://github.com/RIOT-OS/RIOT/blob/master/pkg/lwext4/fs/lwext4_fs.c#L115
I also spotted the lack of explicit NUL-termination after
strncpy()
in RIOT source code at:https://github.com/RIOT-OS/RIOT/blob/master/pkg/lwext4/fs/lwext4_fs.c#L464
Details
If an attacker is able to make
mount_point
at leastCONFIG_EXT4_MAX_MP_NAME
bytes large,strcat()
would write a NUL byte past the end of themp.name
buffer, thus potentially overwriting the next member of theext4_mountpoint
struct described here.Please refer to the marked line below:
Since
sizeof(dirent->name)
is 255 andsizeof(entry->d_name)
is only 32, if an attacker can control the input tostrncpy()
they would be able to create a non NUL-terminated string inentry->d_name
. When such corrupted string is used in other parts of the code, it may cause information leakage or memory corruption.Please refer to the marked line below:
Impact
In my understanding, the impact of these vulnerabilities in this specific case is quite limited. However, all bugs that have the potential to cause memory corruption should be taken seriously and fixed as security bugs.