[gparted] Correctly quote and escape command line argument passed to "sh -c" (#787203)



commit f422052356ab594bac044dbd34fca894d2774625
Author: Pali Rohár <pali rohar gmail com>
Date:   Sun Sep 3 09:49:28 2017 +0200

    Correctly quote and escape command line argument passed to "sh -c" (#787203)
    
    Shell fragments must be properly quoted and escaped to prevent execution
    of unintended commands derived from user controllable data.  For
    example:
    
        $ printf '#!/bin/sh\necho test > out\n' > script.sh
        $ chmod +x script.sh
        $ truncate -s 20M 'jfs;script.sh'
        $ mkfs.jfs -q 'jfs;script.sh'
        $ ls -l out
        ls: cannot access out: No such file or directory
    
        $ sudo PATH=$PWD:$PATH /usr/local/bin/gparted 'jfs;script.sh'
        $ sudo PATH=$PWD:$PATH /usr/local/bin/gparted 'jfs;script.sh'
    
        $ ls -l out
        -rw-r--r-- 1 root root 5 Sep 12 23:11 out
        $ cat out
        test
    
    What is happening is that jfs::set_used_sectors() is using the device
    name 'jfs;script.sh' without quoting it and passing it to the shell to
    execute like this:
        sh -c 'echo dm | jfs_debugfs jfs;script.sh'
    which the shell duly executes as these two commands:
        echo dm | jfs_debugfs jfs
        script.sh
    
    This could be a security related issue as "sh -c" is able to execute
    arbitrary shell commands from the argument if if contains shell special
    characters.  Use Glib::shell_quote() [1] to quote and escape file names
    and whole commands passed to the shell.
    
    [1] Glib::shell_quote(const std::string & unquoted_string)
        https://developer.gnome.org/glibmm/stable/group__ShellUtils.html
        "Quotes a string so that the shell (/bin/sh) will interpret the
        quoted string to mean unquoted_string.
    
        If you pass a filename to the shell, for example, you should first
        quote it with this function."
    
    Bug 787203 - Correctly quote and escape arguments of external programs
                 passed to execute_command()

 src/jfs.cc      |    3 ++-
 src/reiserfs.cc |    6 +++---
 src/xfs.cc      |    5 +++--
 3 files changed, 8 insertions(+), 6 deletions(-)
---
diff --git a/src/jfs.cc b/src/jfs.cc
index e827921..43e9d24 100644
--- a/src/jfs.cc
+++ b/src/jfs.cc
@@ -75,7 +75,8 @@ FS jfs::get_filesystem_support()
 
 void jfs::set_used_sectors( Partition & partition ) 
 {
-       if ( ! Utils::execute_command( "sh -c 'echo dm | jfs_debugfs " + partition.get_path() + "'", output, 
error, true ) )
+       const Glib::ustring jfs_debug_cmd = "echo dm | jfs_debugfs " + Glib::shell_quote( 
partition.get_path() );
+       if ( ! Utils::execute_command( "sh -c " + Glib::shell_quote( jfs_debug_cmd ), output, error, true ) )
        {
                //blocksize
                Glib::ustring::size_type index = output.find( "Block Size:" );
diff --git a/src/reiserfs.cc b/src/reiserfs.cc
index 65c98c8..4daddc5 100644
--- a/src/reiserfs.cc
+++ b/src/reiserfs.cc
@@ -173,9 +173,9 @@ bool reiserfs::resize( const Partition & partition_new, OperationDetail & operat
                size = " -s " + Utils::num_to_str( floor( Utils::sector_to_unit(
                                   partition_new .get_sector_length(), partition_new .sector_size, UNIT_BYTE 
) ) ) ;
        }
-       Glib::ustring cmd = "sh -c 'echo y | resize_reiserfs" + size + " " + partition_new .get_path() + "'" ;
-
-       exit_status = execute_command( cmd, operationdetail ) ;
+       const Glib::ustring resize_cmd = "echo y | resize_reiserfs" + size +
+                                        " " + Glib::shell_quote( partition_new.get_path() );
+       exit_status = execute_command( "sh -c " + Glib::shell_quote( resize_cmd ), operationdetail );
        // NOTE: Neither resize_reiserfs manual page nor the following commit, which first
        // added this check, indicate why exit status 1 also indicates success.  Commit
        // from 2006-05-23:
diff --git a/src/xfs.cc b/src/xfs.cc
index f925737..c01fd1b 100644
--- a/src/xfs.cc
+++ b/src/xfs.cc
@@ -258,8 +258,9 @@ bool xfs::copy( const Partition & src_part,
 
                if ( success )
                {
-                       success &= ! execute_command( "sh -c 'xfsdump -J - \"" + src_mount_point +
-                                                     "\" | xfsrestore -J - \"" + dest_mount_point + "\"'",
+                       const Glib::ustring copy_cmd = "xfsdump -J - " + Glib::shell_quote( src_mount_point ) 
+
+                                                      " | xfsrestore -J - " + Glib::shell_quote( 
dest_mount_point );
+                       success &= ! execute_command( "sh -c " + Glib::shell_quote( copy_cmd ),
                                                      operationdetail,
                                                      EXEC_CHECK_STATUS|EXEC_CANCEL_SAFE|EXEC_PROGRESS_TIMED,
                                                      static_cast<TimedSlot>( sigc::mem_fun( *this, 
&xfs::copy_progress ) ) );


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