Mirrors
/
gparted
mirror of https://gitlab.gnome.org/GNOME/gparted.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

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()
This commit is contained in:
Pali Rohár 2017-09-03 09:49:28 +02:00 committed by Mike Fleetwood
parent e8f0504b13
commit f422052356
3 changed files with 8 additions and 6 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
src/jfs.cc
View File

@ -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:" );

6
src/reiserfs.cc
View File

@ -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:

5
src/xfs.cc
View File

@ -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 ) ) );