equipment: sanitize 'ws_info' loop limits

Instead of a constant or a macro for the maximum
number of 'ws_info' elements the 100 literal was used.

Define MAX_WS_INFO in dive.h and use it everywhere.

Also clamp loops that iterate `ws_info' to MAX_WS_INFO.
Prevents potential out-of-bounds reading, similarly to
the previous commit about 'tank_info'.

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

View file

@ -275,6 +275,7 @@ struct divecomputer {
#define MAX_CYLINDERS (20) #define MAX_CYLINDERS (20)
#define MAX_WEIGHTSYSTEMS (6) #define MAX_WEIGHTSYSTEMS (6)
#define MAX_TANK_INFO (100) #define MAX_TANK_INFO (100)
#define MAX_WS_INFO (100)
#define W_IDX_PRIMARY 0 #define W_IDX_PRIMARY 0
#define W_IDX_SECONDARY 1 #define W_IDX_SECONDARY 1
@ -944,7 +945,7 @@ struct ws_info_t {
const char *name; const char *name;
int grams; int grams;
}; };
extern struct ws_info_t ws_info[100]; extern struct ws_info_t ws_info[MAX_WS_INFO];
extern bool cylinder_nodata(const cylinder_t *cyl); extern bool cylinder_nodata(const cylinder_t *cyl);
extern bool cylinder_none(void *_data); extern bool cylinder_none(void *_data);

View file

@ -43,13 +43,13 @@ void add_weightsystem_description(weightsystem_t *weightsystem)
desc = weightsystem->description; desc = weightsystem->description;
if (!desc) if (!desc)
return; return;
for (i = 0; i < 100 && ws_info[i].name != NULL; i++) { for (i = 0; i < MAX_WS_INFO && ws_info[i].name != NULL; i++) {
if (strcmp(ws_info[i].name, desc) == 0) { if (strcmp(ws_info[i].name, desc) == 0) {
ws_info[i].grams = weightsystem->weight.grams; ws_info[i].grams = weightsystem->weight.grams;
return; return;
} }
} }
if (i < 100) { if (i < MAX_WS_INFO) {
// FIXME: leaked on exit // FIXME: leaked on exit
ws_info[i].name = strdup(desc); ws_info[i].name = strdup(desc);
ws_info[i].grams = weightsystem->weight.grams; ws_info[i].grams = weightsystem->weight.grams;
@ -181,7 +181,7 @@ struct tank_info_t tank_info[100] = {
* We hardcode the most common weight system types * We hardcode the most common weight system types
* This is a bit odd as the weight system types don't usually encode weight * This is a bit odd as the weight system types don't usually encode weight
*/ */
struct ws_info_t ws_info[100] = { struct ws_info_t ws_info[MAX_WS_INFO] = {
{ QT_TRANSLATE_NOOP("gettextFromC", "integrated"), 0 }, { QT_TRANSLATE_NOOP("gettextFromC", "integrated"), 0 },
{ QT_TRANSLATE_NOOP("gettextFromC", "belt"), 0 }, { QT_TRANSLATE_NOOP("gettextFromC", "belt"), 0 },
{ QT_TRANSLATE_NOOP("gettextFromC", "ankle"), 0 }, { QT_TRANSLATE_NOOP("gettextFromC", "ankle"), 0 },

View file

@ -107,7 +107,7 @@ bool WeightModel::setData(const QModelIndex &index, const QVariant &value, int r
if (!ws->description || gettextFromC::tr(ws->description) != vString) { if (!ws->description || gettextFromC::tr(ws->description) != vString) {
// loop over translations to see if one matches // loop over translations to see if one matches
int i = -1; int i = -1;
while (ws_info[++i].name) { while (ws_info[++i].name && i < MAX_WS_INFO) {
if (gettextFromC::tr(ws_info[i].name) == vString) { if (gettextFromC::tr(ws_info[i].name) == vString) {
ws->description = copy_string(ws_info[i].name); ws->description = copy_string(ws_info[i].name);
break; break;

View file

@ -79,8 +79,8 @@ const QString &WSInfoModel::biggerString() const
WSInfoModel::WSInfoModel() : rows(-1) WSInfoModel::WSInfoModel() : rows(-1)
{ {
setHeaderDataStrings(QStringList() << tr("Description") << tr("kg")); setHeaderDataStrings(QStringList() << tr("Description") << tr("kg"));
struct ws_info_t *info = ws_info; struct ws_info_t *info;
for (info = ws_info; info->name; info++, rows++) { for (info = ws_info; info->name && info < ws_info + MAX_WS_INFO; info++, rows++) {
QString wsInfoName = gettextFromC::tr(info->name); QString wsInfoName = gettextFromC::tr(info->name);
if (wsInfoName.count() > biggerEntry.count()) if (wsInfoName.count() > biggerEntry.count())
biggerEntry = wsInfoName; biggerEntry = wsInfoName;
@ -94,11 +94,11 @@ WSInfoModel::WSInfoModel() : rows(-1)
void WSInfoModel::updateInfo() void WSInfoModel::updateInfo()
{ {
struct ws_info_t *info = ws_info; struct ws_info_t *info;
beginRemoveRows(QModelIndex(), 0, this->rows); beginRemoveRows(QModelIndex(), 0, this->rows);
endRemoveRows(); endRemoveRows();
rows = -1; rows = -1;
for (info = ws_info; info->name; info++, rows++) { for (info = ws_info; info->name && info < ws_info + MAX_WS_INFO; info++, rows++) {
QString wsInfoName = gettextFromC::tr(info->name); QString wsInfoName = gettextFromC::tr(info->name);
if (wsInfoName.count() > biggerEntry.count()) if (wsInfoName.count() > biggerEntry.count())
biggerEntry = wsInfoName; biggerEntry = wsInfoName;
@ -117,9 +117,8 @@ void WSInfoModel::update()
endRemoveRows(); endRemoveRows();
rows = -1; rows = -1;
} }
struct ws_info_t *info = ws_info; struct ws_info_t *info;
for (info = ws_info; info->name; info++, rows++) for (info = ws_info; info->name && info < ws_info + MAX_WS_INFO; info++, rows++);
;
if (rows > -1) { if (rows > -1) {
beginInsertRows(QModelIndex(), 0, rows); beginInsertRows(QModelIndex(), 0, rows);