]> git.ipfire.org Git - thirdparty/git.git/commitdiff
Merge branch 'js/fix-open-exec'
authorJohannes Sixt <j6t@kdbg.org>
Tue, 8 Jul 2025 19:00:34 +0000 (21:00 +0200)
committerJohannes Sixt <j6t@kdbg.org>
Tue, 8 Jul 2025 19:00:34 +0000 (21:00 +0200)
This addresses CVE-2025-27613, Gitk can create and truncate a user's
files:

When a user clones an untrusted repository and runs gitk without
additional command arguments, files for which the user has write
permission can be created and truncated. The option "Support per-file
encoding" must have been enabled before in Gitk's Preferences.  This
option is disabled by default.

The same happens when "Show origin of this line" is used in the main
window (regardless of whether "Support per-file encoding" is enabled or
not).

* js/fix-open-exec:
  gitk: sanitize 'open' arguments: revisit recently updated 'open' calls
  gitk: sanitize 'open' arguments: command pipeline
  gitk: collect construction of blameargs into a single conditional
  gitk: sanitize 'open' arguments: simple commands, readable and writable
  gitk: sanitize 'open' arguments: simple commands with redirections
  gitk: sanitize 'open' arguments: simple commands
  gitk: sanitize 'exec' arguments: redirect to process
  gitk: sanitize 'exec' arguments: redirections and background
  gitk: sanitize 'exec' arguments: redirections
  gitk: sanitize 'exec' arguments: 'eval exec'
  gitk: sanitize 'exec' arguments: simple cases
  gitk: have callers of diffcmd supply pipe symbol when necessary
  gitk: treat file names beginning with "|" as relative paths

Signed-off-by: Johannes Sixt <j6t@kdbg.org>
1  2 
gitk

diff --cc gitk
index ecf0f8c052f5841115b9d084fc86ed80c190b354,c3bf6da882ff710f0429ed433e5ad89e14d919ad..5be8b2aeb0e7be24a94c02944b792e7eaf152a9a
--- 1/gitk
--- 2/gitk
+++ b/gitk
@@@ -9,110 -9,92 +9,195 @@@ exec wish "$0" -- "$@
  
  package require Tk
  
 +######################################################################
 +##
 +## Enabling platform-specific code paths
 +
 +proc is_Windows {} {
 +    if {$::tcl_platform(platform) eq {windows}} {
 +        return 1
 +    }
 +    return 0
 +}
 +
 +######################################################################
 +##
 +## PATH lookup
 +
 +if {[is_Windows]} {
 +    set _search_path {}
 +    proc _which {what args} {
 +        global env _search_path
 +
 +        if {$_search_path eq {}} {
 +            set gitguidir [file dirname [info script]]
 +            regsub -all ";" $gitguidir "\\;" gitguidir
 +            set env(PATH) "$gitguidir;$env(PATH)"
 +            set _search_path [split $env(PATH) {;}]
 +            # Skip empty `PATH` elements
 +            set _search_path [lsearch -all -inline -not -exact \
 +                $_search_path ""]
 +        }
 +
 +        if {[lsearch -exact $args -script] >= 0} {
 +            set suffix {}
 +        } else {
 +            set suffix .exe
 +        }
 +
 +        foreach p $_search_path {
 +            set p [file join $p $what$suffix]
 +            if {[file exists $p]} {
 +                return [file normalize $p]
 +            }
 +        }
 +        return {}
 +    }
 +
 +    proc sanitize_command_line {command_line from_index} {
 +        set i $from_index
 +        while {$i < [llength $command_line]} {
 +            set cmd [lindex $command_line $i]
 +            if {[llength [file split $cmd]] < 2} {
 +                set fullpath [_which $cmd]
 +                if {$fullpath eq ""} {
 +                    throw {NOT-FOUND} "$cmd not found in PATH"
 +                }
 +                lset command_line $i $fullpath
 +            }
 +
 +            # handle piped commands, e.g. `exec A | B`
 +            for {incr i} {$i < [llength $command_line]} {incr i} {
 +                if {[lindex $command_line $i] eq "|"} {
 +                    incr i
 +                    break
 +                }
 +            }
 +        }
 +        return $command_line
 +    }
 +
 +    # Override `exec` to avoid unsafe PATH lookup
 +
 +    rename exec real_exec
 +
 +    proc exec {args} {
 +        # skip options
 +        for {set i 0} {$i < [llength $args]} {incr i} {
 +            set arg [lindex $args $i]
 +            if {$arg eq "--"} {
 +                incr i
 +                break
 +            }
 +            if {[string range $arg 0 0] ne "-"} {
 +                break
 +            }
 +        }
 +        set args [sanitize_command_line $args $i]
 +        uplevel 1 real_exec $args
 +    }
 +
 +    # Override `open` to avoid unsafe PATH lookup
 +
 +    rename open real_open
 +
 +    proc open {args} {
 +        set arg0 [lindex $args 0]
 +        if {[string range $arg0 0 0] eq "|"} {
 +            set command_line [string trim [string range $arg0 1 end]]
 +            lset args 0 "| [sanitize_command_line $command_line 0]"
 +        }
 +        uplevel 1 real_open $args
 +    }
 +}
 +
 +# End of safe PATH lookup stuff
  
+ # 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 with redirections
+ # no pipelines are possible
+ # cmd is a list that specifies the command and its arguments
+ # redir is a list that specifies redirections (output, background, constant(!) commands)
+ # calls `exec` and returns its value
+ proc safe_exec_redirect {cmd redir} {
+     eval exec [make_arglist_safe $cmd] $redir
+ }
+ 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
+ }
+ # opens a command pipeline for reading
+ # cmd is a list that specifies the command and its arguments
+ # calls `open` and returns the file id
+ proc safe_open_command {cmd} {
+     open |[make_arglist_safe $cmd] r
+ }
+ # opens a command pipeline for reading and writing
+ # cmd is a list that specifies the command and its arguments
+ # calls `open` and returns the file id
+ proc safe_open_command_rw {cmd} {
+     open |[make_arglist_safe $cmd] r+
+ }
+ # opens a command pipeline for reading with redirections
+ # cmd is a list that specifies the command and its arguments
+ # redir is a list that specifies redirections
+ # calls `open` and returns the file id
+ proc safe_open_command_redirect {cmd redir} {
+     set cmd [make_arglist_safe $cmd]
+     open |[concat $cmd $redir] r
+ }
+ # opens a pipeline with several commands for reading
+ # cmds is a list of lists, each of which specifies a command and its arguments
+ # calls `open` and returns the file id
+ proc safe_open_pipeline {cmds} {
+     set cmd {}
+     foreach subcmd $cmds {
+         set cmd [concat $cmd | [make_arglist_safe $subcmd]]
+     }
+     open $cmd r
+ }
+ # End exec/open wrappers
  proc hasworktree {} {
      return [expr {[exec git rev-parse --is-bare-repository] == "false" &&
                    [exec git rev-parse --is-inside-git-dir] == "false"}]