From 769aca9e956cd4bb7cc97be813968348f5e7f3d2 Mon Sep 17 00:00:00 2001 From: "Lubomir I. Ivanov" Date: Tue, 19 Jun 2018 03:19:56 +0300 Subject: 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 --- qt-models/tankinfomodel.cpp | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) (limited to 'qt-models') diff --git a/qt-models/tankinfomodel.cpp b/qt-models/tankinfomodel.cpp index 49e42830c..0a54cd305 100644 --- a/qt-models/tankinfomodel.cpp +++ b/qt-models/tankinfomodel.cpp @@ -91,7 +91,7 @@ TankInfoModel::TankInfoModel() : rows(-1) { setHeaderDataStrings(QStringList() << tr("Description") << tr("ml") << tr("bar")); 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); if (infoName.count() > biggerEntry.count()) biggerEntry = infoName; @@ -111,8 +111,7 @@ void TankInfoModel::update() rows = -1; } 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) { beginInsertRows(QModelIndex(), 0, rows); -- cgit v1.2.3-70-g09d2