equipment: sanitize 'tank_info' loop limits

In a number of places the global 'tank_info' array
is being iterated based on a 'tank_info[idx].name != NULL'
condition.

This is dangerous because if the user has added a lot of tanks,
such loops can reach 'tank_info[MAX_TANK_INFO]'. This is an
out of bounds read and if the 'name' pointer there happens to be
non-NULL, passing that address to a peace of code that tries
to read it (like strlen()) would either SIGSEGV or have undefined
behavior.

Clamp all loops that iterate 'tank_info' to MAX_TANK_INFO.

Signed-off-by: Lubomir I. Ivanov <neolit123@gmail.com>
This commit is contained in:
Lubomir I. Ivanov 2018-06-19 03:19:56 +03:00 committed by Dirk Hohndel
parent a5380bb741
commit 769aca9e95
3 changed files with 4 additions and 5 deletions

View file

@ -209,7 +209,7 @@ void fill_default_cylinder(cylinder_t *cyl)
if (!cyl_name) if (!cyl_name)
return; return;
while (ti->name != NULL) { while (ti->name != NULL && ti < tank_info + MAX_TANK_INFO) {
if (strcmp(ti->name, cyl_name) == 0) if (strcmp(ti->name, cyl_name) == 0)
break; break;
ti++; ti++;

View file

@ -52,7 +52,7 @@ void PreferencesDefaults::refreshSettings()
ui->localDefaultFile->setChecked(prefs.default_file_behavior == LOCAL_DEFAULT_FILE); ui->localDefaultFile->setChecked(prefs.default_file_behavior == LOCAL_DEFAULT_FILE);
ui->default_cylinder->clear(); ui->default_cylinder->clear();
for (int i = 0; tank_info[i].name != NULL; i++) { for (int i = 0; tank_info[i].name != NULL && i < MAX_TANK_INFO; i++) {
ui->default_cylinder->addItem(tank_info[i].name); ui->default_cylinder->addItem(tank_info[i].name);
if (prefs.default_cylinder && strcmp(tank_info[i].name, prefs.default_cylinder) == 0) if (prefs.default_cylinder && strcmp(tank_info[i].name, prefs.default_cylinder) == 0)
ui->default_cylinder->setCurrentIndex(i); ui->default_cylinder->setCurrentIndex(i);

View file

@ -91,7 +91,7 @@ TankInfoModel::TankInfoModel() : rows(-1)
{ {
setHeaderDataStrings(QStringList() << tr("Description") << tr("ml") << tr("bar")); setHeaderDataStrings(QStringList() << tr("Description") << tr("ml") << tr("bar"));
struct tank_info_t *info = tank_info; struct tank_info_t *info = tank_info;
for (info = tank_info; info->name; info++, rows++) { for (info = tank_info; info->name && info < tank_info + MAX_TANK_INFO; info++, rows++) {
QString infoName = gettextFromC::tr(info->name); QString infoName = gettextFromC::tr(info->name);
if (infoName.count() > biggerEntry.count()) if (infoName.count() > biggerEntry.count())
biggerEntry = infoName; biggerEntry = infoName;
@ -111,8 +111,7 @@ void TankInfoModel::update()
rows = -1; rows = -1;
} }
struct tank_info_t *info = tank_info; struct tank_info_t *info = tank_info;
for (info = tank_info; info->name; info++, rows++) for (info = tank_info; info->name && info < tank_info + MAX_TANK_INFO; info++, rows++);
;
if (rows > -1) { if (rows > -1) {
beginInsertRows(QModelIndex(), 0, rows); beginInsertRows(QModelIndex(), 0, rows);