[gparted] Fix crash in Create New Partition dialog when changing type (#101)



commit e91db19e30576cc7b897c1c5f8a939bedcee6ad0
Author: Mike Fleetwood <mike fleetwood googlemail com>
Date:   Sun Feb 28 13:12:34 2021 +0000

    Fix crash in Create New Partition dialog when changing type (#101)
    
    On an MSDOS partitioned drive, open the Create New Partition dialog and
    change "created as" from Primary Partition to Extended Partition and
    back to Primary Partition.  On Fedora and RHEL/CentOS 8, which builds
    packages with FORTIFY_SOURCE [1][2] and GLIBXX_Assertions [3][4]
    enabled, GParted will crash.
    
    Run GParted built with the default compilation options under valgrind
    and repeat the test.  Multiple out of bounds reads are reported like
    this:
      # valgrind --track-origins=yes ./gpartedbin
      ...
      ==232613== Invalid read of size 8
      ==232613==    at 0x441AF6: GParted::Dialog_Partition_New::combobox_changed(bool) 
(Dialog_Partition_New.cc:354)
      ==232613==    by 0x443DBD: sigc::bound_mem_functor1<void, GParted::Dialog_Partition_New, 
bool>::operator()(bool const&) const (mem_fun.h:2066)
    
    Coming from Dialog_Partition_New.cc:
      328  void Dialog_Partition_New::combobox_changed(bool type)
      329  {
      ...
      351      // combo_filesystem and combo_alignment
      352      if ( ! type )
      353      {
    > 354          fs = FILESYSTEMS[combo_filesystem.get_active_row_number()];
    
    When the partition type is changed to Extended the file system is forced
    to be "Extended" too.  This is done in ::combobox_changed() method by
    modifying combo_filesystem to add "Extended", making that the selected
    item and setting the widget as inactive.
    
    Then when the partition type is changed back to primary the file system
    combobox is returned to it's previous state.  This is done by first
    removing the last "Extended" item, making the widget active and setting
    the selected item.  However as "Extended" is the currently selected
    item, removing it forces their to be no selected item and triggers a
    change to combo_filesystem triggering a recursive call to
    ::combobox_changed() where combo_filesystem.get_active_row_number()
    returns -1 (no selection) [5] and on line 354 the code accesses item -1
    of the FILESYSTEMS[] vector.
    
    Fix by setting the new combo_filesystem selection before removing the
    currently selected "Extended" item.  This has the added benefit of only
    triggering a change to combo_filesystem once when the default item is
    selected rather than twice when the currently "Extended" item is removed
    and again when the default item is selected.
    
    [1] [Fedora] Security Features, Compile Time Buffer Checks
        (FORTIFY_SOURCE)
        https://fedoraproject.org/wiki/Security_Features#Compile_Time_Buffer_Checks_.28FORTIFY_SOURCE.29
    
    [2] Enhance application security with FORTIFY_SOURCE
        https://access.redhat.com/blogs/766093/posts/1976213
    
    [3] Security Features Matrix (GLIBXX_Assertions)
        https://fedoraproject.org/wiki/Security_Features_Matrix
    
    [4] GParted 1.2.0-1.fc33 package build.log for Fedora 33
        https://kojipkgs.fedoraproject.org/packages/gparted/1.2.0/1.fc33/data/logs/x86_64/build.log
        CXXFLAGS='-O2 -g ... -Wp,-D_FORTIFY_SOURCE=2
        -Wp,-D_GLIBCXX_ASSERTIONS ...'
    
    [5] gtkmm: Gtk::ComboBox Class Reference, get_active_row_number()
        https://developer.gnome.org/gtkmm/stable/classGtk_1_1ComboBox.html#a53531bc041b5a460826babb8496c363b
    
    Closes #101 - Crash changing Partition type in "Create new partition"
                  dialog

 src/Dialog_Partition_New.cc | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
---
diff --git a/src/Dialog_Partition_New.cc b/src/Dialog_Partition_New.cc
index 873124b9..5dcdefe6 100644
--- a/src/Dialog_Partition_New.cc
+++ b/src/Dialog_Partition_New.cc
@@ -342,9 +342,9 @@ void Dialog_Partition_New::combobox_changed(bool type)
                else if (combo_type.get_active_row_number() != TYPE_EXTENDED      &&
                         combo_filesystem.items().size()    == FILESYSTEMS.size()   )
                {
+                       combo_filesystem.set_active(first_creatable_fs);
                        combo_filesystem.items().erase(combo_filesystem.items().back());
                        combo_filesystem.set_sensitive(true);
-                       combo_filesystem.set_active(first_creatable_fs);
                }
        }
        


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]