Fix obscure segfault when resizing terminal

This was a very obscure segmentation fault I
stumbled across while testing the drive
selection window. I confirmed it exists
in all versions at least going back prior
to 0.24 and maybe a lot earlier.

It was a very tricky bug to track down as you
had to do some very specific things to cause it to
occur. Probably most people would not have seen
it, however for those that did, this was what you
needed to do to trigger it.

1. First you had to be wiping more than one drive.

2. Then you needed to scroll down to the bottom
drive in the list.

3. You then needed to minimise the vertical height
of the terminal so you could no longer see the
selected drive.

4. Then you needed to expand the vertical height
of the terminal. This last step would trigger a
segmentation fault and crash nwipe.

So, the theory. nwipe uses four variables to
allow you to scroll through multiple displayed
drives. These four variables are:

count = the number of enumerated drives.

slots = the number of available horizontal lines
        to display the drives. The number of slots
        varies depending upon vertical resizing
        of the terminal. So slots is calculated
        in real time.

focus = which drive the GUI '>' pointer is pointing to.

offset = A value between 0 and slots that describes
         what drives are displayed in the available
         slots.

offset is then used along with i in a for loop to index
the drive contexts. This is where the segfault
occurred.

The calculation failed to take correct account of a varying
number of slots so in the condition described above the
offset would create an index that was out of bounds.

So first I fixed the miss calculation and second I placed
an if statement that acts as a bounds checker so if
ever this code is changed by someone in the future and
they break the calculation the bounds checker 'if' statement
will log the details of the error and prevent a segfault which
is far easier to debug.
This commit is contained in:
PartialVolume
2020-04-01 19:50:04 +01:00
parent 62beaca8ac
commit d140feb94b

132
src/gui.c
View File

@@ -523,7 +523,7 @@ void nwipe_gui_select( int count, nwipe_context_t** c )
int focus = 0;
/* A generic loop variable. */
int i;
int i = 0;
/* User input buffer. */
int keystroke;
@@ -551,6 +551,38 @@ void nwipe_gui_select( int count, nwipe_context_t** c )
/* Less two lines for the box and two lines for padding. */
slots = wlines - 4;
if( slots < 0 )
{
slots = 0;
}
/* The code here adjusts the offset value, required when the terminal is resized vertically */
if( slots > count )
{
offset = 0;
}
else
{
if( focus >= count )
{
/* The focus is already at the last element. */
focus = count - 1;
}
if( focus < 0 )
{
/* The focus is already at the last element. */
focus = 0;
}
}
if( count >= slots && slots > 0 )
{
offset = focus + 1 - slots;
if( offset < 0 )
{
offset = 0;
}
}
/* Clear the main window, necessary when switching selections such as method etc */
werase( main_window );
@@ -601,42 +633,59 @@ void nwipe_gui_select( int count, nwipe_context_t** c )
waddch( main_window, ' ' );
}
switch( c[i + offset]->select )
/* In the event of the offset value so how becoming invalid, this if statement will prevent a segfault
* and the else part will log the out of bounds values for debugging */
if( i + offset >= 0 && i + offset < count )
{
case NWIPE_SELECT_TRUE:
wprintw( main_window, " [wipe] %i. %s", ( i + offset + 1 ), c[i + offset]->device_label );
break;
switch( c[i + offset]->select )
{
case NWIPE_SELECT_TRUE:
case NWIPE_SELECT_FALSE:
/* Print an element that is not selected. */
wprintw( main_window, " [ ] %i. %s", ( i + offset + 1 ), c[i + offset]->device_label );
break;
wprintw( main_window, " [wipe] %i. %s", ( i + offset + 1 ), c[i + offset]->device_label );
break;
case NWIPE_SELECT_TRUE_PARENT:
case NWIPE_SELECT_FALSE:
/* Print an element that is not selected. */
wprintw( main_window, " [ ] %i. %s", ( i + offset + 1 ), c[i + offset]->device_label );
break;
/* This element will be wiped when its parent is wiped. */
wprintw( main_window, " [****] %i. %s", ( i + offset + 1 ), c[i + offset]->device_label );
break;
case NWIPE_SELECT_TRUE_PARENT:
case NWIPE_SELECT_FALSE_CHILD:
/* This element will be wiped when its parent is wiped. */
wprintw( main_window, " [****] %i. %s", ( i + offset + 1 ), c[i + offset]->device_label );
break;
/* We can't wipe this element because it has a child that is being wiped. */
wprintw( main_window, " [----] %i. %s", ( i + offset + 1 ), c[i + offset]->device_label );
break;
case NWIPE_SELECT_FALSE_CHILD:
case NWIPE_SELECT_DISABLED:
/* We can't wipe this element because it has a child that is being wiped. */
wprintw( main_window, " [----] %i. %s", ( i + offset + 1 ), c[i + offset]->device_label );
break;
/* We don't know how to wipe this device. (Iomega Zip drives.) */
wprintw( main_window, " [????] %s", "Unrecognized Device" );
break;
case NWIPE_SELECT_DISABLED:
default:
/* We don't know how to wipe this device. (Iomega Zip drives.) */
wprintw( main_window, " [????] %s", "Unrecognized Device" );
break;
/* TODO: Handle the sanity error. */
break;
default:
} /* switch select */
/* TODO: Handle the sanity error. */
break;
} /* switch select */
}
else
{
nwipe_log( NWIPE_LOG_DEBUG,
"GUI.c,nwipe_gui_select(), scroll, array index out of bounds, i=%u, count=%u, slots=%u, "
"focus=%u, offset=%u",
i,
count,
slots,
focus,
offset );
}
} /* for */
@@ -672,10 +721,9 @@ void nwipe_gui_select( int count, nwipe_context_t** c )
* which wastes CPU cycles.
*/
validkeyhit = 0;
do
{
validkeyhit = 0;
timeout( 250 ); // block getch() for 250ms.
keystroke = getch(); // Get user input.
timeout( -1 ); // Switch back to blocking mode.
@@ -686,6 +734,8 @@ void nwipe_gui_select( int count, nwipe_context_t** c )
case 'j':
case 'J':
validkeyhit = 1;
/* Increment the focus. */
focus += 1;
@@ -702,7 +752,6 @@ void nwipe_gui_select( int count, nwipe_context_t** c )
offset += 1;
break;
}
validkeyhit = 1;
break;
@@ -710,6 +759,8 @@ void nwipe_gui_select( int count, nwipe_context_t** c )
case 'k':
case 'K':
validkeyhit = 1;
/* Decrement the focus. */
focus -= 1;
@@ -726,7 +777,6 @@ void nwipe_gui_select( int count, nwipe_context_t** c )
offset -= 1;
break;
}
validkeyhit = 1;
break;
@@ -734,6 +784,8 @@ void nwipe_gui_select( int count, nwipe_context_t** c )
case 10:
case ' ':
validkeyhit = 1;
/* TODO: This block should be made into a function. */
if( c[focus]->select == NWIPE_SELECT_TRUE )
@@ -800,8 +852,6 @@ void nwipe_gui_select( int count, nwipe_context_t** c )
} /* else super-enable */
validkeyhit = 1;
break;
} /* if NWIPE_SELECT_TRUE */
@@ -848,54 +898,58 @@ void nwipe_gui_select( int count, nwipe_context_t** c )
} /* else super-deselect */
validkeyhit = 1;
break;
} /* if NWIPE_SELECT_FALSE */
/* TODO: Explain to the user why they can't change this. */
validkeyhit = 1;
break;
case 'm':
case 'M':
validkeyhit = 1;
/* Run the method dialog. */
nwipe_gui_method();
validkeyhit = 1;
break;
case 'p':
case 'P':
validkeyhit = 1;
/* Run the PRNG dialog. */
nwipe_gui_prng();
validkeyhit = 1;
break;
case 'r':
case 'R':
validkeyhit = 1;
/* Run the rounds dialog. */
nwipe_gui_rounds();
validkeyhit = 1;
break;
case 'v':
case 'V':
validkeyhit = 1;
/* Run the option dialog. */
nwipe_gui_verify();
validkeyhit = 1;
break;
case 'b':
case 'B':
validkeyhit = 1;
/* Run the noblank dialog. */
nwipe_gui_noblank();
validkeyhit = 1;
break;
case 'S':