Merge branch 'js/fix-open-exec-git'

This addresses CVE-2025-46835, Git GUI can create and overwrite a user's files: When a user clones an untrusted repository and is tricked into editing a file located in a maliciously named directory in the repository, then Git GUI can create and overwrite files for which the user has write permission. * js/fix-open-exec-git: git-gui: sanitize 'exec' arguments: convert new 'cygpath' calls git-gui: do not mistake command arguments as redirection operators git-gui: introduce function git_redir for git calls with redirections git-gui: pass redirections as separate argument to git_read git-gui: pass redirections as separate argument to _open_stdout_stderr git-gui: convert git_read*, git_write to be non-variadic git-gui: use git_read in githook_read git-gui: break out a separate function git_read_nice git-gui: remove option --stderr from git_read git-gui: sanitize 'exec' arguments: background git-gui: sanitize 'exec' arguments: simple cases git-gui: treat file names beginning with "|" as relative paths git-gui: remove git config --list handling for git < 1.5.3 git-gui: remove HEAD detachment implementation for git < 1.5.3 git-gui: remove Tcl 8.4 workaround on 2>@1 redirection Signed-off-by: Johannes Sixt <j6t@kdbg.org>
2025-07-08 21:22:00 +02:00 · 2025-07-08 21:22:00 +02:00 · 3f07230844
parent 88125ffe70 a437f5bc93
commit 3f07230844
20 changed files with 217 additions and 233 deletions
--- a/git-gui.sh
+++ b/git-gui.sh
@ -199,6 +199,56 @@ if {[is_Windows]} {
 	}
 }

+# Wrap exec/open to sanitize arguments
+
+# unsafe arguments begin with redirections or the pipe or background operators
+proc is_arg_unsafe {arg} {
+	regexp {^([<|>&]|2>)} $arg
+}
+
+proc make_arg_safe {arg} {
+	if {[is_arg_unsafe $arg]} {
+		set arg [file join . $arg]
+	}
+	return $arg
+}
+
+proc make_arglist_safe {arglist} {
+	set res {}
+	foreach arg $arglist {
+		lappend res [make_arg_safe $arg]
+	}
+	return $res
+}
+
+# executes one command
+# no redirections or pipelines are possible
+# cmd is a list that specifies the command and its arguments
+# calls `exec` and returns its value
+proc safe_exec {cmd} {
+	eval exec [make_arglist_safe $cmd]
+}
+
+# executes one command in the background
+# no redirections or pipelines are possible
+# cmd is a list that specifies the command and its arguments
+# calls `exec` and returns its value
+proc safe_exec_bg {cmd} {
+	eval exec [make_arglist_safe $cmd] &
+}
+
+proc safe_open_file {filename flags} {
+	# a file name starting with "|" would attempt to run a process
+	# but such a file name must be treated as a relative path
+	# hide the "|" behind "./"
+	if {[string index $filename 0] eq "|"} {
+		set filename [file join . $filename]
+	}
+	open $filename $flags
+}
+
+# End exec/open wrappers
+
 ######################################################################
 ##
 ## locate our library
@ -299,11 +349,11 @@ unset oguimsg

 if {[tk windowingsystem] eq "aqua"} {
 	catch {
-		exec osascript -e [format {
+		safe_exec [list osascript -e [format {
 			tell application "System Events"
 				set frontmost of processes whose unix id is %d to true
 			end tell
-		} [pid]]
+		} [pid]]]
 	}
 }

@ -343,7 +393,7 @@ if {[string match @@* $_shellpath]} {
 }

 if {[is_Windows]} {
-	set _shellpath [exec cygpath -m $_shellpath]
+	set _shellpath [safe_exec [list cygpath -m $_shellpath]]
 }

 if {![file executable $_shellpath] || \
@ -545,7 +595,7 @@ proc _git_cmd {name} {
 			# Tcl on Windows doesn't know it.
 			#
 			set p [gitexec git-$name]
-			set f [open $p r]
+			set f [safe_open_file $p r]
 			set s [gets $f]
 			close $f

@ -582,6 +632,7 @@ proc _git_cmd {name} {
 # how to run.
 proc open_cmd_pipe {cmd path} {
 	set run [list [shellpath] -c "$cmd \"\$0\"" $path]
+	set run [make_arglist_safe $run]
 	return [open |$run r]
 }

@ -591,7 +642,7 @@ proc _lappend_nice {cmd_var} {

 	if {![info exists _nice]} {
 		set _nice [_which nice]
-		if {[catch {exec $_nice git version}]} {
+		if {[catch {safe_exec [list $_nice git version]}]} {
 			set _nice {}
 		} elseif {[is_Windows] && [file dirname $_nice] ne [file dirname $::_git]} {
 			set _nice {}
@ -603,7 +654,11 @@ proc _lappend_nice {cmd_var} {
 }

 proc git {args} {
-	set fd [eval [list git_read] $args]
+	git_redir $args {}
+}
+
+proc git_redir {cmd redir} {
+	set fd [git_read $cmd $redir]
 	fconfigure $fd -translation binary -encoding utf-8
 	set result [string trimright [read $fd] "\n"]
 	close $fd
@ -613,88 +668,47 @@ proc git {args} {
 	return $result
 }

-proc _open_stdout_stderr {cmd} {
-	_trace_exec $cmd
+proc safe_open_command {cmd {redir {}}} {
+	set cmd [make_arglist_safe $cmd]
+	_trace_exec [concat $cmd $redir]
 	if {[catch {
-			set fd [open [concat [list | ] $cmd] r]
-		} err]} {
-		if {   [lindex $cmd end] eq {2>@1}
-		    && $err eq {can not find channel named "1"}
-			} {
-			# Older versions of Tcl 8.4 don't have this 2>@1 IO
-			# redirect operator.  Fallback to |& cat for those.
-			# The command was not actually started, so its safe
-			# to try to start it a second time.
-			#
-			set fd [open [concat \
-				[list | ] \
-				[lrange $cmd 0 end-1] \
-				[list |& cat] \
-				] r]
-		} else {
-			error $err
-		}
+		set fd [open [concat [list | ] $cmd $redir] r]
+	} err]} {
+		error $err
 	}
 	fconfigure $fd -eofchar {}
 	return $fd
 }

-proc git_read {args} {
-	set opt [list]
+proc git_read {cmd {redir {}}} {
+	set cmdp [_git_cmd [lindex $cmd 0]]
+	set cmd [lrange $cmd 1 end]

-	while {1} {
-		switch -- [lindex $args 0] {
-		--nice {
-			_lappend_nice opt
-		}
-
-		--stderr {
-			lappend args 2>@1
-		}
-
-		default {
-			break
-		}
-
-		}
-
-		set args [lrange $args 1 end]
-	}
-
-	set cmdp [_git_cmd [lindex $args 0]]
-	set args [lrange $args 1 end]
-
-	return [_open_stdout_stderr [concat $opt $cmdp $args]]
+	return [safe_open_command [concat $cmdp $cmd] $redir]
 }

-proc git_write {args} {
+proc git_read_nice {cmd} {
 	set opt [list]

-	while {1} {
-		switch -- [lindex $args 0] {
-		--nice {
-			_lappend_nice opt
-		}
+	_lappend_nice opt

-		default {
-			break
-		}
+	set cmdp [_git_cmd [lindex $cmd 0]]
+	set cmd [lrange $cmd 1 end]

-		}
+	return [safe_open_command [concat $opt $cmdp $cmd]]
+}

-		set args [lrange $args 1 end]
-	}
+proc git_write {cmd} {
+	set cmd [make_arglist_safe $cmd]
+	set cmdp [_git_cmd [lindex $cmd 0]]
+	set cmd [lrange $cmd 1 end]

-	set cmdp [_git_cmd [lindex $args 0]]
-	set args [lrange $args 1 end]
-
-	_trace_exec [concat $opt $cmdp $args]
-	return [open [concat [list | ] $opt $cmdp $args] w]
+	_trace_exec [concat $cmdp $cmd]
+	return [open [concat [list | ] $cmdp $cmd] w]
 }

 proc githook_read {hook_name args} {
-	set cmd [concat git hook run --ignore-missing $hook_name -- $args 2>@1]
-	return [_open_stdout_stderr $cmd]
+	git_read [concat [list hook run --ignore-missing $hook_name --] $args] [list 2>@1]
 }

 proc kill_file_process {fd} {
@ -702,9 +716,9 @@ proc kill_file_process {fd} {

 	catch {
 		if {[is_Windows]} {
-			exec taskkill /pid $process
+			safe_exec [list taskkill /pid $process]
 		} else {
-			exec kill $process
+			safe_exec [list kill $process]
 		}
 	}
 }
@ -730,7 +744,7 @@ proc sq {value} {
 proc load_current_branch {} {
 	global current_branch is_detached

-	set fd [open [gitdir HEAD] r]
+	set fd [safe_open_file [gitdir HEAD] r]
 	fconfigure $fd -translation binary -encoding utf-8
 	if {[gets $fd ref] < 1} {
 		set ref {}
@ -1100,7 +1114,7 @@ You are using [git-version]:
 ## configure our library

 set idx [file join $oguilib tclIndex]
-if {[catch {set fd [open $idx r]} err]} {
+if {[catch {set fd [safe_open_file $idx r]} err]} {
 	catch {wm withdraw .}
 	tk_messageBox \
 		-icon error \
@ -1138,53 +1152,30 @@ unset -nocomplain idx fd
 ##
 ## config file parsing

-git-version proc _parse_config {arr_name args} {
-	>= 1.5.3 {
-		upvar $arr_name arr
-		array unset arr
-		set buf {}
-		catch {
-			set fd_rc [eval \
-				[list git_read config] \
-				$args \
-				[list --null --list]]
-			fconfigure $fd_rc -translation binary -encoding utf-8
-			set buf [read $fd_rc]
-			close $fd_rc
-		}
-		foreach line [split $buf "\0"] {
-			if {[regexp {^([^\n]+)\n(.*)$} $line line name value]} {
-				if {[is_many_config $name]} {
-					lappend arr($name) $value
-				} else {
-					set arr($name) $value
-				}
-			} elseif {[regexp {^([^\n]+)$} $line line name]} {
-				# no value given, but interpreting them as
-				# boolean will be handled as true
-				set arr($name) {}
-			}
-		}
+proc _parse_config {arr_name args} {
+	upvar $arr_name arr
+	array unset arr
+	set buf {}
+	catch {
+		set fd_rc [git_read \
+			[concat config \
+			$args \
+			--null --list]]
+		fconfigure $fd_rc -translation binary -encoding utf-8
+		set buf [read $fd_rc]
+		close $fd_rc
 	}
-	default {
-		upvar $arr_name arr
-		array unset arr
-		catch {
-			set fd_rc [eval [list git_read config --list] $args]
-			while {[gets $fd_rc line] >= 0} {
-				if {[regexp {^([^=]+)=(.*)$} $line line name value]} {
-					if {[is_many_config $name]} {
-						lappend arr($name) $value
-					} else {
-						set arr($name) $value
-					}
-				} elseif {[regexp {^([^=]+)$} $line line name]} {
-					# no value given, but interpreting them as
-					# boolean will be handled as true
-					set arr($name) {}
-				}
+	foreach line [split $buf "\0"] {
+		if {[regexp {^([^\n]+)\n(.*)$} $line line name value]} {
+			if {[is_many_config $name]} {
+				lappend arr($name) $value
+			} else {
+				set arr($name) $value
 			}
-			close $fd_rc
+		} elseif {[regexp {^([^\n]+)$} $line line name]} {
+			# no value given, but interpreting them as
+			# boolean will be handled as true
+			set arr($name) {}
 		}
 	}
 }
@ -1459,7 +1450,7 @@ proc repository_state {ctvar hdvar mhvar} {
 	set merge_head [gitdir MERGE_HEAD]
 	if {[file exists $merge_head]} {
 		set ct merge
-		set fd_mh [open $merge_head r]
+		set fd_mh [safe_open_file $merge_head r]
 		while {[gets $fd_mh line] >= 0} {
 			lappend mh $line
 		}
@ -1478,7 +1469,7 @@ proc PARENT {} {
 		return $p
 	}
 	if {$empty_tree eq {}} {
-		set empty_tree [git mktree << {}]
+		set empty_tree [git_redir [list mktree] [list << {}]]
 	}
 	return $empty_tree
 }
@ -1537,12 +1528,12 @@ proc rescan {after {honor_trustmtime 1}} {
 	} else {
 		set rescan_active 1
 		ui_status [mc "Refreshing file status..."]
-		set fd_rf [git_read update-index \
+		set fd_rf [git_read [list update-index \
 			-q \
 			--unmerged \
 			--ignore-missing \
 			--refresh \
-			]
+			]]
 		fconfigure $fd_rf -blocking 0 -translation binary
 		fileevent $fd_rf readable \
 			[list rescan_stage2 $fd_rf $after]
@ -1582,11 +1573,11 @@ proc rescan_stage2 {fd after} {
 	set rescan_active 2
 	ui_status [mc "Scanning for modified files ..."]
 	if {[git-version >= "1.7.2"]} {
-		set fd_di [git_read diff-index --cached --ignore-submodules=dirty -z [PARENT]]
+		set fd_di [git_read [list diff-index --cached --ignore-submodules=dirty -z [PARENT]]]
 	} else {
-		set fd_di [git_read diff-index --cached -z [PARENT]]
+		set fd_di [git_read [list diff-index --cached -z [PARENT]]]
 	}
-	set fd_df [git_read diff-files -z]
+	set fd_df [git_read [list diff-files -z]]

 	fconfigure $fd_di -blocking 0 -translation binary -encoding binary
 	fconfigure $fd_df -blocking 0 -translation binary -encoding binary
@ -1595,7 +1586,7 @@ proc rescan_stage2 {fd after} {
 	fileevent $fd_df readable [list read_diff_files $fd_df $after]

 	if {[is_config_true gui.displayuntracked]} {
-		set fd_lo [eval git_read ls-files --others -z $ls_others]
+		set fd_lo [git_read [concat ls-files --others -z $ls_others]]
 		fconfigure $fd_lo -blocking 0 -translation binary -encoding binary
 		fileevent $fd_lo readable [list read_ls_others $fd_lo $after]
 		incr rescan_active
@ -1607,7 +1598,7 @@ proc load_message {file {encoding {}}} {

 	set f [gitdir $file]
 	if {[file isfile $f]} {
-		if {[catch {set fd [open $f r]}]} {
+		if {[catch {set fd [safe_open_file $f r]}]} {
 			return 0
 		}
 		fconfigure $fd -eofchar {}
@ -1631,23 +1622,23 @@ proc run_prepare_commit_msg_hook {} {
 	# it will be .git/MERGE_MSG (merge), .git/SQUASH_MSG (squash), or an
 	# empty file but existent file.

-	set fd_pcm [open [gitdir PREPARE_COMMIT_MSG] a]
+	set fd_pcm [safe_open_file [gitdir PREPARE_COMMIT_MSG] a]

 	if {[file isfile [gitdir MERGE_MSG]]} {
 		set pcm_source "merge"
-		set fd_mm [open [gitdir MERGE_MSG] r]
+		set fd_mm [safe_open_file [gitdir MERGE_MSG] r]
 		fconfigure $fd_mm -encoding utf-8
 		puts -nonewline $fd_pcm [read $fd_mm]
 		close $fd_mm
 	} elseif {[file isfile [gitdir SQUASH_MSG]]} {
 		set pcm_source "squash"
-		set fd_sm [open [gitdir SQUASH_MSG] r]
+		set fd_sm [safe_open_file [gitdir SQUASH_MSG] r]
 		fconfigure $fd_sm -encoding utf-8
 		puts -nonewline $fd_pcm [read $fd_sm]
 		close $fd_sm
 	} elseif {[file isfile [get_config commit.template]]} {
 		set pcm_source "template"
-		set fd_sm [open [get_config commit.template] r]
+		set fd_sm [safe_open_file [get_config commit.template] r]
 		fconfigure $fd_sm -encoding utf-8
 		puts -nonewline $fd_pcm [read $fd_sm]
 		close $fd_sm
@ -2237,7 +2228,7 @@ proc do_gitk {revs {is_submodule false}} {
 			unset env(GIT_DIR)
 			unset env(GIT_WORK_TREE)
 		}
-		eval exec $cmd $revs "--" "--" &
+		safe_exec_bg [concat $cmd $revs "--" "--"]

 		set env(GIT_DIR) $_gitdir
 		set env(GIT_WORK_TREE) $_gitworktree
@ -2274,7 +2265,7 @@ proc do_git_gui {} {
 		set pwd [pwd]
 		cd $current_diff_path

-		eval exec $exe gui &
+		safe_exec_bg [concat $exe gui]

 		set env(GIT_DIR) $_gitdir
 		set env(GIT_WORK_TREE) $_gitworktree
@ -2305,16 +2296,18 @@ proc get_explorer {} {

 proc do_explore {} {
 	global _gitworktree
-	set explorer [get_explorer]
-	eval exec $explorer [list [file nativename $_gitworktree]] &
+	set cmd [get_explorer]
+	lappend cmd [file nativename $_gitworktree]
+	safe_exec_bg $cmd
 }

 # Open file relative to the working tree by the default associated app.
 proc do_file_open {file} {
 	global _gitworktree
-	set explorer [get_explorer]
+	set cmd [get_explorer]
 	set full_file_path [file join $_gitworktree $file]
-	exec $explorer [file nativename $full_file_path] &
+	lappend cmd [file nativename $full_file_path]
+	safe_exec_bg $cmd
 }

 set is_quitting 0
@ -2348,7 +2341,7 @@ proc do_quit {{rc {1}}} {
 			if {![string match amend* $commit_type]
 				&& $msg ne {}} {
 				catch {
-					set fd [open $save w]
+					set fd [safe_open_file $save w]
 					fconfigure $fd -encoding utf-8
 					puts -nonewline $fd $msg
 					close $fd
@ -2792,15 +2785,15 @@ if {![is_bare]} {

 if {[is_Windows]} {
 	# Use /git-bash.exe if available
-	set _git_bash [exec cygpath -m /git-bash.exe]
+	set _git_bash [safe_exec [list cygpath -m /git-bash.exe]]
 	if {[file executable $_git_bash]} {
-		set _bash_cmdline [list "Git Bash" $_git_bash &]
+		set _bash_cmdline [list "Git Bash" $_git_bash]
 	} else {
-		set _bash_cmdline [list "Git Bash" bash --login -l &]
+		set _bash_cmdline [list "Git Bash" bash --login -l]
 	}
 	.mbar.repository add command \
 		-label [mc "Git Bash"] \
-		-command {eval exec [list [_which cmd] /c start] $_bash_cmdline}
+		-command {safe_exec_bg [concat [list [_which cmd] /c start] $_bash_cmdline]}
 	unset _git_bash
 }

@ -4110,7 +4103,7 @@ if {[winfo exists $ui_comm]} {
 				}
 			} elseif {$m} {
 				catch {
-					set fd [open [gitdir GITGUI_BCK] w]
+					set fd [safe_open_file [gitdir GITGUI_BCK] w]
 					fconfigure $fd -encoding utf-8
 					puts -nonewline $fd $msg
 					close $fd
--- a/lib/blame.tcl
+++ b/lib/blame.tcl
@ -481,14 +481,14 @@ method _load {jump} {
 		if {$do_textconv ne 0} {
 			set fd [open_cmd_pipe $textconv $path]
 		} else {
-			set fd [open $path r]
+			set fd [safe_open_file $path r]
 		}
 		fconfigure $fd -eofchar {}
 	} else {
 		if {$do_textconv ne 0} {
-			set fd [git_read cat-file --textconv "$commit:$path"]
+			set fd [git_read [list cat-file --textconv "$commit:$path"]]
 		} else {
-			set fd [git_read cat-file blob "$commit:$path"]
+			set fd [git_read [list cat-file blob "$commit:$path"]]
 		}
 	}
 	fconfigure $fd \
@ -617,7 +617,7 @@ method _exec_blame {cur_w cur_d options cur_s} {
 	}

 	lappend options -- $path
-	set fd [eval git_read --nice blame $options]
+	set fd [git_read_nice [concat blame $options]]
 	fconfigure $fd -blocking 0 -translation lf -encoding utf-8
 	fileevent $fd readable [cb _read_blame $fd $cur_w $cur_d]
 	set current_fd $fd
@ -986,7 +986,7 @@ method _showcommit {cur_w lno} {
 		if {[catch {set msg $header($cmit,message)}]} {
 			set msg {}
 			catch {
-				set fd [git_read cat-file commit $cmit]
+				set fd [git_read [list cat-file commit $cmit]]
 				fconfigure $fd -encoding binary -translation lf
 				# By default commits are assumed to be in utf-8
 				set enc utf-8
@ -1134,7 +1134,7 @@ method _blameparent {} {
 		} else {
 			set diffcmd [list diff-tree --unified=0 $cparent $cmit -- $new_path]
 		}
-		if {[catch {set fd [eval git_read $diffcmd]} err]} {
+		if {[catch {set fd [git_read $diffcmd]} err]} {
 			$status_operation stop [mc "Unable to display parent"]
 			error_popup [strcat [mc "Error loading diff:"] "\n\n$err"]
 			return
--- a/lib/branch.tcl
+++ b/lib/branch.tcl
@ -7,7 +7,7 @@ proc load_all_heads {} {
 	set rh refs/heads
 	set rh_len [expr {[string length $rh] + 1}]
 	set all_heads [list]
-	set fd [git_read for-each-ref --format=%(refname) $rh]
+	set fd [git_read [list for-each-ref --format=%(refname) $rh]]
 	fconfigure $fd -translation binary -encoding utf-8
 	while {[gets $fd line] > 0} {
 		if {!$some_heads_tracking || ![is_tracking_branch $line]} {
@ -21,10 +21,10 @@ proc load_all_heads {} {

 proc load_all_tags {} {
 	set all_tags [list]
-	set fd [git_read for-each-ref \
+	set fd [git_read [list for-each-ref \
 		--sort=-taggerdate \
 		--format=%(refname) \
-		refs/tags]
+		refs/tags]]
 	fconfigure $fd -translation binary -encoding utf-8
 	while {[gets $fd line] > 0} {
 		if {![regsub ^refs/tags/ $line {} name]} continue
--- a/lib/browser.tcl
+++ b/lib/browser.tcl
@ -196,7 +196,7 @@ method _ls {tree_id {name {}}} {
 	lappend browser_stack [list $tree_id $name]
 	$w conf -state disabled

-	set fd [git_read ls-tree -z $tree_id]
+	set fd [git_read [list ls-tree -z $tree_id]]
 	fconfigure $fd -blocking 0 -translation binary -encoding utf-8
 	fileevent $fd readable [cb _read $fd]
 }
--- a/lib/checkout_op.tcl
+++ b/lib/checkout_op.tcl
@ -304,12 +304,12 @@ The rescan will be automatically started now.
 		_readtree $this
 	} else {
 		ui_status [mc "Refreshing file status..."]
-		set fd [git_read update-index \
+		set fd [git_read [list update-index \
 			-q \
 			--unmerged \
 			--ignore-missing \
 			--refresh \
-			]
+			]]
 		fconfigure $fd -blocking 0 -translation binary
 		fileevent $fd readable [cb _refresh_wait $fd]
 	}
@ -345,14 +345,15 @@ method _readtree {} {
 		[mc "Updating working directory to '%s'..." [_name $this]] \
 		[mc "files checked out"]]

-	set fd [git_read --stderr read-tree \
+	set fd [git_read [list read-tree \
 		-m \
 		-u \
 		-v \
 		--exclude-per-directory=.gitignore \
 		$HEAD \
 		$new_hash \
-		]
+		] \
+		[list 2>@1]]
 	fconfigure $fd -blocking 0 -translation binary
 	fileevent $fd readable [cb _readtree_wait $fd $status_bar_operation]
 }
@ -510,18 +511,8 @@ method _update_repo_state {} {
 	delete_this
 }

-git-version proc _detach_HEAD {log new} {
-	>= 1.5.3 {
-		git update-ref --no-deref -m $log HEAD $new
-	}
-	default {
-		set p [gitdir HEAD]
-		file delete $p
-		set fd [open $p w]
-		fconfigure $fd -translation lf -encoding utf-8
-		puts $fd $new
-		close $fd
-	}
+proc _detach_HEAD {log new} {
+	git update-ref --no-deref -m $log HEAD $new
 }

 method _confirm_reset {cur} {
@ -582,7 +573,7 @@ method _confirm_reset {cur} {
 	pack $w.buttons.cancel -side right -padx 5
 	pack $w.buttons -side bottom -fill x -pady 10 -padx 10

-	set fd [git_read rev-list --pretty=oneline $cur ^$new_hash]
+	set fd [git_read [list rev-list --pretty=oneline $cur ^$new_hash]]
 	while {[gets $fd line] > 0} {
 		set abbr [string range $line 0 7]
 		set subj [string range $line 41 end]
--- a/lib/choose_repository.tcl
+++ b/lib/choose_repository.tcl
@ -641,8 +641,8 @@ method _do_clone2 {} {
 			set pwd [pwd]
 			if {[catch {
 				file mkdir [gitdir objects info]
-				set f_in [open [file join $objdir info alternates] r]
-				set f_cp [open [gitdir objects info alternates] w]
+				set f_in [safe_open_file [file join $objdir info alternates] r]
+				set f_cp [safe_open_file [gitdir objects info alternates] w]
 				fconfigure $f_in -translation binary -encoding binary
 				fconfigure $f_cp -translation binary -encoding binary
 				cd $objdir
@ -727,7 +727,7 @@ method _do_clone2 {} {
 			[cb _do_clone_tags]
 	}
 	shared {
-		set fd [open [gitdir objects info alternates] w]
+		set fd [safe_open_file [gitdir objects info alternates] w]
 		fconfigure $fd -translation binary
 		puts $fd $objdir
 		close $fd
@ -760,8 +760,8 @@ method _copy_files {objdir tocopy} {
 	}
 	foreach p $tocopy {
 		if {[catch {
-				set f_in [open [file join $objdir $p] r]
-				set f_cp [open [file join .git objects $p] w]
+				set f_in [safe_open_file [file join $objdir $p] r]
+				set f_cp [safe_open_file [file join .git objects $p] w]
 				fconfigure $f_in -translation binary -encoding binary
 				fconfigure $f_cp -translation binary -encoding binary

@ -818,12 +818,12 @@ method _clone_refs {} {
 		error_popup [mc "Not a Git repository: %s" [file tail $origin_url]]
 		return 0
 	}
-	set fd_in [git_read for-each-ref \
+	set fd_in [git_read [list for-each-ref \
 		--tcl \
-		{--format=list %(refname) %(objectname) %(*objectname)}]
+		{--format=list %(refname) %(objectname) %(*objectname)}]]
 	cd $pwd

-	set fd [open [gitdir packed-refs] w]
+	set fd [safe_open_file [gitdir packed-refs] w]
 	fconfigure $fd -translation binary
 	puts $fd "# pack-refs with: peeled"
 	while {[gets $fd_in line] >= 0} {
@ -877,7 +877,7 @@ method _do_clone_full_end {ok} {

 		set HEAD {}
 		if {[file exists [gitdir FETCH_HEAD]]} {
-			set fd [open [gitdir FETCH_HEAD] r]
+			set fd [safe_open_file [gitdir FETCH_HEAD] r]
 			while {[gets $fd line] >= 0} {
 				if {[regexp "^(.{40})\t\t" $line line HEAD]} {
 					break
@ -953,13 +953,14 @@ method _do_clone_checkout {HEAD} {
 		[mc "files"]]

 	set readtree_err {}
-	set fd [git_read --stderr read-tree \
+	set fd [git_read [list read-tree \
 		-m \
 		-u \
 		-v \
 		HEAD \
 		HEAD \
-		]
+		] \
+		[list 2>@1]]
 	fconfigure $fd -blocking 0 -translation binary
 	fileevent $fd readable [cb _readtree_wait $fd]
 }
--- a/lib/choose_rev.tcl
+++ b/lib/choose_rev.tcl
@ -146,14 +146,14 @@ constructor _new {path unmerged_only title} {
 	append fmt { %(*subject)}
 	append fmt {]}
 	set all_refn [list]
-	set fr_fd [git_read for-each-ref \
+	set fr_fd [git_read [list for-each-ref \
 		--tcl \
 		--sort=-taggerdate \
 		--format=$fmt \
 		refs/heads \
 		refs/remotes \
 		refs/tags \
-		]
+		]]
 	fconfigure $fr_fd -translation lf -encoding utf-8
 	while {[gets $fr_fd line] > 0} {
 		set line [eval $line]
@ -176,7 +176,7 @@ constructor _new {path unmerged_only title} {
 	close $fr_fd

 	if {$unmerged_only} {
-		set fr_fd [git_read rev-list --all ^$::HEAD]
+		set fr_fd [git_read [list rev-list --all ^$::HEAD]]
 		while {[gets $fr_fd sha1] > 0} {
 			if {[catch {set rlst $cmt_refn($sha1)}]} continue
 			foreach refn $rlst {
@ -579,7 +579,7 @@ method _reflog_last {name} {

 	set last {}
 	if {[catch {set last [file mtime [gitdir $name]]}]
-	&& ![catch {set g [open [gitdir logs $name] r]}]} {
+	&& ![catch {set g [safe_open_file [gitdir logs $name] r]}]} {
 		fconfigure $g -translation binary
 		while {[gets $g line] >= 0} {
 			if {[regexp {> ([1-9][0-9]*) } $line line when]} {
--- a/lib/commit.tcl
+++ b/lib/commit.tcl
@ -27,7 +27,7 @@ You are currently in the middle of a merge that has not been fully completed.  Y
 	if {[catch {
 			set name ""
 			set email ""
-			set fd [git_read cat-file commit $curHEAD]
+			set fd [git_read [list cat-file commit $curHEAD]]
 			fconfigure $fd -encoding binary -translation lf
 			# By default commits are assumed to be in utf-8
 			set enc utf-8
@ -234,7 +234,7 @@ A good commit message has the following format:
 	# -- Build the message file.
 	#
 	set msg_p [gitdir GITGUI_EDITMSG]
-	set msg_wt [open $msg_p w]
+	set msg_wt [safe_open_file $msg_p w]
 	fconfigure $msg_wt -translation lf
 	setup_commit_encoding $msg_wt
 	puts $msg_wt $msg
@ -334,7 +334,7 @@ proc commit_commitmsg_wait {fd_ph curHEAD msg_p} {

 proc commit_writetree {curHEAD msg_p} {
 	ui_status [mc "Committing changes..."]
-	set fd_wt [git_read write-tree]
+	set fd_wt [git_read [list write-tree]]
 	fileevent $fd_wt readable \
 		[list commit_committree $fd_wt $curHEAD $msg_p]
 }
@ -359,7 +359,7 @@ proc commit_committree {fd_wt curHEAD msg_p} {
 	# -- Verify this wasn't an empty change.
 	#
 	if {$commit_type eq {normal}} {
-		set fd_ot [git_read cat-file commit $PARENT]
+		set fd_ot [git_read [list cat-file commit $PARENT]]
 		fconfigure $fd_ot -encoding binary -translation lf
 		set old_tree [gets $fd_ot]
 		close $fd_ot
@ -397,8 +397,8 @@ A rescan will be automatically started now.
 	foreach p [concat $PARENT $MERGE_HEAD] {
 		lappend cmd -p $p
 	}
-	lappend cmd <$msg_p
-	if {[catch {set cmt_id [eval git $cmd]} err]} {
+	set msgtxt [list <$msg_p]
+	if {[catch {set cmt_id [git_redir $cmd $msgtxt]} err]} {
 		catch {file delete $msg_p}
 		error_popup [strcat [mc "commit-tree failed:"] "\n\n$err"]
 		ui_status [mc "Commit failed."]
@ -418,7 +418,7 @@ A rescan will be automatically started now.
 	if {$commit_type ne {normal}} {
 		append reflogm " ($commit_type)"
 	}
-	set msg_fd [open $msg_p r]
+	set msg_fd [safe_open_file $msg_p r]
 	setup_commit_encoding $msg_fd 1
 	gets $msg_fd subject
 	close $msg_fd
--- a/lib/console.tcl
+++ b/lib/console.tcl
@ -92,10 +92,9 @@ method _init {} {

 method exec {cmd {after {}}} {
 	if {[lindex $cmd 0] eq {git}} {
-		set fd_f [eval git_read --stderr [lrange $cmd 1 end]]
+		set fd_f [git_read [lrange $cmd 1 end] [list 2>@1]]
 	} else {
-		lappend cmd 2>@1
-		set fd_f [_open_stdout_stderr $cmd]
+		set fd_f [safe_open_command $cmd [list 2>@1]]
 	}
 	fconfigure $fd_f -blocking 0 -translation binary -encoding [encoding system]
 	fileevent $fd_f readable [cb _read $fd_f $after]
--- a/lib/database.tcl
+++ b/lib/database.tcl
@ -3,7 +3,7 @@

 proc do_stats {} {
 	global use_ttk NS
-	set fd [git_read count-objects -v]
+	set fd [git_read [list count-objects -v]]
 	while {[gets $fd line] > 0} {
 		if {[regexp {^([^:]+): (\d+)$} $line _ name value]} {
 			set stats($name) $value
--- a/lib/diff.tcl
+++ b/lib/diff.tcl
@ -191,7 +191,7 @@ proc show_other_diff {path w m cont_info} {
 					set sz [string length $content]
 				}
 				file {
-					set fd [open $path r]
+					set fd [safe_open_file $path r]
 					fconfigure $fd \
 						-eofchar {} \
 						-encoding [get_path_encoding $path]
@ -215,7 +215,7 @@ proc show_other_diff {path w m cont_info} {
 			$ui_diff insert end \
 				"* [mc "Git Repository (subproject)"]\n" \
 				d_info
-		} elseif {![catch {set type [exec file $path]}]} {
+		} elseif {![catch {set type [safe_exec [list file $path]]}]} {
 			set n [string length $path]
 			if {[string equal -length $n $path $type]} {
 				set type [string range $type $n end]
@ -327,7 +327,7 @@ proc start_show_diff {cont_info {add_opts {}}} {
 		}
 	}

-	if {[catch {set fd [eval git_read --nice $cmd]} err]} {
+	if {[catch {set fd [git_read_nice $cmd]} err]} {
 		set diff_active 0
 		unlock_index
 		ui_status [mc "Unable to display %s" [escape_path $path]]
@ -603,7 +603,7 @@ proc apply_or_revert_hunk {x y revert} {

 	if {[catch {
 		set enc [get_path_encoding $current_diff_path]
-		set p [eval git_write $apply_cmd]
+		set p [git_write $apply_cmd]
 		fconfigure $p -translation binary -encoding $enc
 		puts -nonewline $p $wholepatch
 		close $p} err]} {
@ -839,7 +839,7 @@ proc apply_or_revert_range_or_line {x y revert} {

 	if {[catch {
 		set enc [get_path_encoding $current_diff_path]
-		set p [eval git_write $apply_cmd]
+		set p [git_write $apply_cmd]
 		fconfigure $p -translation binary -encoding $enc
 		puts -nonewline $p $current_diff_header
 		puts -nonewline $p $wholepatch
@ -876,7 +876,7 @@ proc undo_last_revert {} {

 	if {[catch {
 		set enc $last_revert_enc
-		set p [eval git_write $apply_cmd]
+		set p [git_write $apply_cmd]
 		fconfigure $p -translation binary -encoding $enc
 		puts -nonewline $p $last_revert
 		close $p} err]} {
--- a/lib/index.tcl
+++ b/lib/index.tcl
@ -75,7 +75,7 @@ proc update_indexinfo {msg path_list after} {
 	if {$batch > 25} {set batch 25}

 	set status_bar_operation [$::main_status start $msg [mc "files"]]
-	set fd [git_write update-index -z --index-info]
+	set fd [git_write [list update-index -z --index-info]]
 	fconfigure $fd \
 		-blocking 0 \
 		-buffering full \
@ -144,7 +144,7 @@ proc update_index {msg path_list after} {
 	if {$batch > 25} {set batch 25}

 	set status_bar_operation [$::main_status start $msg [mc "files"]]
-	set fd [git_write update-index --add --remove -z --stdin]
+	set fd [git_write [list update-index --add --remove -z --stdin]]
 	fconfigure $fd \
 		-blocking 0 \
 		-buffering full \
@ -218,13 +218,13 @@ proc checkout_index {msg path_list after capture_error} {
 	if {$batch > 25} {set batch 25}

 	set status_bar_operation [$::main_status start $msg [mc "files"]]
-	set fd [git_write checkout-index \
+	set fd [git_write [list checkout-index \
 		--index \
 		--quiet \
 		--force \
 		-z \
 		--stdin \
-		]
+		]]
 	fconfigure $fd \
 		-blocking 0 \
 		-buffering full \
--- a/lib/merge.tcl
+++ b/lib/merge.tcl
@ -93,7 +93,7 @@ method _start {} {
 	set spec [$w_rev get_tracking_branch]
 	set cmit [$w_rev get_commit]

-	set fh [open [gitdir FETCH_HEAD] w]
+	set fh [safe_open_file [gitdir FETCH_HEAD] w]
 	fconfigure $fh -translation lf
 	if {$spec eq {}} {
 		set remote .
@ -118,7 +118,7 @@ method _start {} {
 		set cmd [list git]
 		lappend cmd merge
 		lappend cmd --strategy=recursive
-		lappend cmd [git fmt-merge-msg <[gitdir FETCH_HEAD]]
+		lappend cmd [git_redir [list fmt-merge-msg] [list <[gitdir FETCH_HEAD]]]
 		lappend cmd HEAD
 		lappend cmd $name
 	}
@ -239,7 +239,7 @@ Continue with resetting the current changes?"]
 	}

 	if {[ask_popup $op_question] eq {yes}} {
-		set fd [git_read --stderr read-tree --reset -u -v HEAD]
+		set fd [git_read [list read-tree --reset -u -v HEAD] [list 2>@1]]
 		fconfigure $fd -blocking 0 -translation binary
 		set status_bar_operation [$::main_status \
 			start \
--- a/lib/mergetool.tcl
+++ b/lib/mergetool.tcl
@ -88,7 +88,7 @@ proc merge_load_stages {path cont} {
 	set merge_stages(3) {}
 	set merge_stages_buf {}

-	set merge_stages_fd [eval git_read ls-files -u -z -- {$path}]
+	set merge_stages_fd [git_read [list ls-files -u -z -- $path]]

 	fconfigure $merge_stages_fd -blocking 0 -translation binary -encoding binary
 	fileevent $merge_stages_fd readable [list read_merge_stages $merge_stages_fd $cont]
@ -310,7 +310,7 @@ proc merge_tool_get_stages {target stages} {
 	foreach fname $stages {
 		if {$merge_stages($i) eq {}} {
 			file delete $fname
-			catch { close [open $fname w] }
+			catch { close [safe_open_file $fname w] }
 		} else {
 			# A hack to support autocrlf properly
 			git checkout-index -f --stage=$i -- $target
@ -360,9 +360,9 @@ proc merge_tool_start {cmdline target backup stages} {

 	# Force redirection to avoid interpreting output on stderr
 	# as an error, and launch the tool
-	lappend cmdline {2>@1}
+	set redir [list {2>@1}]

-	if {[catch { set mtool_fd [_open_stdout_stderr $cmdline] } err]} {
+	if {[catch { set mtool_fd [safe_open_command $cmdline $redir] } err]} {
 		delete_temp_files $mtool_tmpfiles
 		error_popup [mc "Could not start the merge tool:\n\n%s" $err]
 		return
--- a/lib/remote.tcl
+++ b/lib/remote.tcl
@ -32,7 +32,7 @@ proc all_tracking_branches {} {
 	}

 	if {$pat ne {}} {
-		set fd [eval git_read for-each-ref --format=%(refname) $cmd]
+		set fd [git_read [concat for-each-ref --format=%(refname) $cmd]]
 		while {[gets $fd n] > 0} {
 			foreach spec $pat {
 				set dst [string range [lindex $spec 0] 0 end-2]
@ -75,7 +75,7 @@ proc load_all_remotes {} {

 		foreach name $all_remotes {
 			catch {
-				set fd [open [file join $rm_dir $name] r]
+				set fd [safe_open_file [file join $rm_dir $name] r]
 				while {[gets $fd line] >= 0} {
 					if {[regexp {^URL:[ 	]*(.+)$} $line line url]} {
 						set remote_url($name) $url
@ -145,7 +145,7 @@ proc add_fetch_entry {r} {
 		}
 	} else {
 		catch {
-			set fd [open [gitdir remotes $r] r]
+			set fd [safe_open_file [gitdir remotes $r] r]
 			while {[gets $fd n] >= 0} {
 				if {[regexp {^Pull:[ \t]*([^:]+):} $n]} {
 					set enable 1
@ -182,7 +182,7 @@ proc add_push_entry {r} {
 		}
 	} else {
 		catch {
-			set fd [open [gitdir remotes $r] r]
+			set fd [safe_open_file [gitdir remotes $r] r]
 			while {[gets $fd n] >= 0} {
 				if {[regexp {^Push:[ \t]*([^:]+):} $n]} {
 					set enable 1
--- a/lib/remote_branch_delete.tcl
+++ b/lib/remote_branch_delete.tcl
@ -308,7 +308,7 @@ method _load {cache uri} {
 		set full_list [list]
 		set head_cache($cache) [list]
 		set full_cache($cache) [list]
-		set active_ls [git_read ls-remote $uri]
+		set active_ls [git_read [list ls-remote $uri]]
 		fconfigure $active_ls \
 			-blocking 0 \
 			-translation lf \
--- a/lib/shortcut.tcl
+++ b/lib/shortcut.tcl
@ -30,8 +30,8 @@ proc do_cygwin_shortcut {} {
 	global argv0 _gitworktree oguilib

 	if {[catch {
-		set desktop [exec cygpath \
-			--desktop]
+		set desktop [safe_exec [list cygpath \
+			--desktop]]
 		}]} {
 			set desktop .
 	}
@ -50,14 +50,14 @@ proc do_cygwin_shortcut {} {
 					"CHERE_INVOKING=1 \
 					source /etc/profile; \
 					git gui"}
-				exec /bin/mkshortcut.exe \
+				safe_exec [list /bin/mkshortcut.exe \
 					--arguments $shargs \
 					--desc "git-gui on $repodir" \
 					--icon $oguilib/git-gui.ico \
 					--name $fn \
 					--show min \
 					--workingdir $repodir \
-					/bin/sh.exe
+					/bin/sh.exe]
 			} err]} {
 			error_popup [strcat [mc "Cannot write shortcut:"] "\n\n$err"]
 		}
@ -83,7 +83,7 @@ proc do_macosx_app {} {

 				file mkdir $MacOS

-				set fd [open [file join $Contents Info.plist] w]
+				set fd [safe_open_file [file join $Contents Info.plist] w]
 				puts $fd {<?xml version="1.0" encoding="UTF-8"?>
 <!DOCTYPE plist PUBLIC "-//Apple Computer//DTD PLIST 1.0//EN" "http://www.apple.com/DTDs/PropertyList-1.0.dtd">
 <plist version="1.0">
@ -108,7 +108,7 @@ proc do_macosx_app {} {
 </plist>}
 				close $fd

-				set fd [open $exe w]
+				set fd [safe_open_file $exe w]
 				puts $fd "#!/bin/sh"
 				foreach name [lsort [array names env]] {
 					set value $env($name)
--- a/lib/sshkey.tcl
+++ b/lib/sshkey.tcl
@ -7,7 +7,7 @@ proc find_ssh_key {} {
 		~/.ssh/id_rsa.pub ~/.ssh/identity.pub
 	} {
 		if {[file exists $name]} {
-			set fh    [open $name r]
+			set fh    [safe_open_file $name r]
 			set cont  [read $fh]
 			close $fh
 			return [list $name $cont]
@ -86,7 +86,7 @@ proc make_ssh_key {w} {
 	set cmdline [list [shellpath] -c \
 		{echo | ssh-keygen -q -t rsa -f ~/.ssh/id_rsa 2>&1}]

-	if {[catch { set sshkey_fd [_open_stdout_stderr $cmdline] } err]} {
+	if {[catch { set sshkey_fd [safe_open_command $cmdline] } err]} {
 		error_popup [mc "Could not start ssh-keygen:\n\n%s" $err]
 		return
 	}
--- a/lib/tools.tcl
+++ b/lib/tools.tcl
@ -130,8 +130,7 @@ proc tools_exec {fullname} {
 }

 proc tools_run_silent {cmd after} {
-	lappend cmd 2>@1
-	set fd [_open_stdout_stderr $cmd]
+	set fd [safe_open_command $cmd [list 2>@1]]

 	fconfigure $fd -blocking 0 -translation binary
 	fileevent $fd readable [list tools_consume_input $fd $after]
--- a/lib/win32.tcl
+++ b/lib/win32.tcl
@ -2,11 +2,11 @@
 # Copyright (C) 2007 Shawn Pearce

 proc win32_read_lnk {lnk_path} {
-	return [exec cscript.exe \
+	return [safe_exec [list cscript.exe \
 		/E:jscript \
 		/nologo \
 		[file join $::oguilib win32_shortcut.js] \
-		$lnk_path]
+		$lnk_path]]
 }

 proc win32_create_lnk {lnk_path lnk_exec lnk_dir} {
@ -15,12 +15,13 @@ proc win32_create_lnk {lnk_path lnk_exec lnk_dir} {
 	set lnk_args [lrange $lnk_exec 1 end]
 	set lnk_exec [lindex $lnk_exec 0]

-	eval [list exec wscript.exe \
+	set cmd [list wscript.exe \
 		/E:jscript \
 		/nologo \
 		[file nativename [file join $oguilib win32_shortcut.js]] \
 		$lnk_path \
 		[file nativename [file join $oguilib git-gui.ico]] \
 		$lnk_dir \
-		$lnk_exec] $lnk_args
+		$lnk_exec]
+	safe_exec [concat $cmd $lnk_args]
 }