On 28 April 2016 at 11:25, Rick Walsh <[email protected]> wrote:
> Hi,
>
> I was just looking at the layout of the planner, and realized that I
> couldn't find a use for the use field in the available gasses table.
>
> We determine whether or not a segment is closed circuit by whether a
> setpoint has been set in the dive planner points table. It doesn't look
> like we actually use the cylinder use value.
>
> Can it be removed?
>
> Furthermore, the only way I could find to set the value is to type in the
> number (0-7) corresponding to the enum. There's no drop-down of options,
> and typing the name of the value doesn't work. There's also a lovely
> segfault when I enter the value 8 (or any value greater than 7). If I enter
> a negative number, the text becomes one of the default dive tags. Yay
> memory. Segfault with value 8.
>
> (gdb) bt
> #0 0x00007ffff0e5db3a in strlen () from /lib64/libc.so.6
> #1 0x00007ffff1a097c0 in QByteArray::QByteArray(char const*, int) () from
> /lib64/libQt5Core.so.5
> #2 0x00000000006d0105 in gettextFromC::trGettext (this=0xcf5d50, text=0xd1
> <error: Cannot access memory at address 0xd1>)
> at /home/rick/src/subsurface/core/gettextfromc.cpp:7
> #3 0x000000000065470a in CylindersModel::data (this=0xec1020, index=...,
> role=2) at /home/rick/src/subsurface/qt-models/cylindermodel.cpp:138
^ that would be a:
cylindermodel.cpp:137:
case USE:
ret =
gettextFromC::instance()->trGettext(cylinderuse_text[cyl->cylinder_use]);
where "cyl" is of type "cylinder_t" and "cylinder_use" is of type
"enum cylinderuse".
hmm, but "enum cylinderuse" doesn't have 7 elements, it has 3 (or
NUM_GAS_USE)...
as defined in dive.h:54:
enum cylinderuse {OC_GAS, DILUENT, OXYGEN, NUM_GAS_USE};
are you sure there are 7 elements and if so where are they defined?
a patch that can fix the potential buffer overflow of N elements is attached.
but it's probably a good thing to also limit the user on the UI side -
i.e. once a bad value is entered that field should be auto-adjusted to
a good value.
lubomir
--
From 981b9840db9cd4820c870fb553491480383f46f6 Mon Sep 17 00:00:00 2001
From: "Lubomir I. Ivanov" <[email protected]>
Date: Thu, 28 Apr 2016 17:33:05 +0300
Subject: [PATCH] CylindersModel: clamp the "cylinderuse" values
If the value for "use" is larger than the number of elements in
"enum cylinderuse", later CylindersModel::data() can request a
string in the lines of cylinderuse_text[cyl->cylinder_use], which
can SIGSEGV.
Signed-off-by: Lubomir I. Ivanov <[email protected]>
---
qt-models/cylindermodel.cpp | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/qt-models/cylindermodel.cpp b/qt-models/cylindermodel.cpp
index b1ce0be..6e0abf0 100644
--- a/qt-models/cylindermodel.cpp
+++ b/qt-models/cylindermodel.cpp
@@ -273,7 +273,10 @@ bool CylindersModel::setData(const QModelIndex &index, const QVariant &value, in
break;
case USE:
if (CHANGED()) {
- cyl->cylinder_use = (enum cylinderuse)vString.toInt();
+ int use = vString.toInt();
+ if (use > NUM_GAS_USE - 1)
+ use = 0;
+ cyl->cylinder_use = (enum cylinderuse)use;
changed = true;
}
break;
--
1.7.11.msysgit.0
_______________________________________________
subsurface mailing list
[email protected]
http://lists.subsurface-divelog.org/cgi-bin/mailman/listinfo/subsurface
