Skip to content

Commit 8976500

Browse files
rhansengitster
rhansen
authored and
gitster
committed
git-prompt.sh: don't put unsanitized branch names in $PS1
Both bash and zsh subject the value of PS1 to parameter expansion, command substitution, and arithmetic expansion. Rather than include the raw, unescaped branch name in PS1 when running in two- or three-argument mode, construct PS1 to reference a variable that holds the branch name. Because the shells do not recursively expand, this avoids arbitrary code execution by specially-crafted branch names such as '$(IFS=_;cmd=sudo_rm_-rf_/;$cmd)'. Signed-off-by: Richard Hansen <[email protected]> Signed-off-by: Junio C Hamano <[email protected]>
1 parent 7bbc4e8 commit 8976500

File tree

2 files changed

+54
-24
lines changed

2 files changed

+54
-24
lines changed

‎contrib/completion/git-prompt.sh

Lines changed: 32 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -207,7 +207,18 @@ __git_ps1_show_upstream ()
207207
p=" u+${count#* }-${count% *}" ;;
208208
esac
209209
if [[ -n "$count" && -n "$name" ]]; then
210-
p="$p $(git rev-parse --abbrev-ref "$upstream" 2>/dev/null)"
210+
__git_ps1_upstream_name=$(git rev-parse \
211+
--abbrev-ref "$upstream" 2>/dev/null)
212+
if [ $pcmode = yes ]; then
213+
# see the comments around the
214+
# __git_ps1_branch_name variable below
215+
p="$p \${__git_ps1_upstream_name}"
216+
else
217+
p="$p ${__git_ps1_upstream_name}"
218+
# not needed anymore; keep user's
219+
# environment clean
220+
unset __git_ps1_upstream_name
221+
fi
211222
fi
212223
fi
213224

@@ -438,8 +449,27 @@ __git_ps1 ()
438449
__git_ps1_colorize_gitstring
439450
fi
440451

452+
b=${b##refs/heads/}
453+
if [ $pcmode = yes ]; then
454+
# In pcmode (and only pcmode) the contents of
455+
# $gitstring are subject to expansion by the shell.
456+
# Avoid putting the raw ref name in the prompt to
457+
# protect the user from arbitrary code execution via
458+
# specially crafted ref names (e.g., a ref named
459+
# '$(IFS=_;cmd=sudo_rm_-rf_/;$cmd)' would execute
460+
# 'sudo rm -rf /' when the prompt is drawn). Instead,
461+
# put the ref name in a new global variable (in the
462+
# __git_ps1_* namespace to avoid colliding with the
463+
# user's environment) and reference that variable from
464+
# PS1.
465+
__git_ps1_branch_name=$b
466+
# note that the $ is escaped -- the variable will be
467+
# expanded later (when it's time to draw the prompt)
468+
b="\${__git_ps1_branch_name}"
469+
fi
470+
441471
local f="$w$i$s$u"
442-
local gitstring="$c${b##refs/heads/}${f:+$z$f}$r$p"
472+
local gitstring="$c$b${f:+$z$f}$r$p"
443473

444474
if [ $pcmode = yes ]; then
445475
if [ "${__git_printf_supports_v-}" != yes ]; then

‎t/t9903-bash-prompt.sh

Lines changed: 22 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -452,67 +452,67 @@ test_expect_success 'prompt - format string starting with dash' '
452452
'
453453

454454
test_expect_success 'prompt - pc mode' '
455-
printf "BEFORE: (master):AFTER" >expected &&
455+
printf "BEFORE: (\${__git_ps1_branch_name}):AFTER\\nmaster" >expected &&
456456
printf "" >expected_output &&
457457
(
458458
__git_ps1 "BEFORE:" ":AFTER" >"$actual" &&
459459
test_cmp expected_output "$actual" &&
460-
printf "%s" "$PS1" >"$actual"
460+
printf "%s\\n%s" "$PS1" "${__git_ps1_branch_name}" >"$actual"
461461
) &&
462462
test_cmp expected "$actual"
463463
'
464464

465465
test_expect_success 'prompt - bash color pc mode - branch name' '
466-
printf "BEFORE: (${c_green}master${c_clear}):AFTER" >expected &&
466+
printf "BEFORE: (${c_green}\${__git_ps1_branch_name}${c_clear}):AFTER\\nmaster" >expected &&
467467
(
468468
GIT_PS1_SHOWCOLORHINTS=y &&
469469
__git_ps1 "BEFORE:" ":AFTER" >"$actual"
470-
printf "%s" "$PS1" >"$actual"
470+
printf "%s\\n%s" "$PS1" "${__git_ps1_branch_name}" >"$actual"
471471
) &&
472472
test_cmp expected "$actual"
473473
'
474474

475475
test_expect_success 'prompt - bash color pc mode - detached head' '
476-
printf "BEFORE: (${c_red}(%s...)${c_clear}):AFTER" $(git log -1 --format="%h" b1^) >expected &&
476+
printf "BEFORE: (${c_red}\${__git_ps1_branch_name}${c_clear}):AFTER\\n(%s...)" $(git log -1 --format="%h" b1^) >expected &&
477477
git checkout b1^ &&
478478
test_when_finished "git checkout master" &&
479479
(
480480
GIT_PS1_SHOWCOLORHINTS=y &&
481481
__git_ps1 "BEFORE:" ":AFTER" &&
482-
printf "%s" "$PS1" >"$actual"
482+
printf "%s\\n%s" "$PS1" "${__git_ps1_branch_name}" >"$actual"
483483
) &&
484484
test_cmp expected "$actual"
485485
'
486486

487487
test_expect_success 'prompt - bash color pc mode - dirty status indicator - dirty worktree' '
488-
printf "BEFORE: (${c_green}master${c_clear} ${c_red}*${c_clear}):AFTER" >expected &&
488+
printf "BEFORE: (${c_green}\${__git_ps1_branch_name}${c_clear} ${c_red}*${c_clear}):AFTER\\nmaster" >expected &&
489489
echo "dirty" >file &&
490490
test_when_finished "git reset --hard" &&
491491
(
492492
GIT_PS1_SHOWDIRTYSTATE=y &&
493493
GIT_PS1_SHOWCOLORHINTS=y &&
494494
__git_ps1 "BEFORE:" ":AFTER" &&
495-
printf "%s" "$PS1" >"$actual"
495+
printf "%s\\n%s" "$PS1" "${__git_ps1_branch_name}" >"$actual"
496496
) &&
497497
test_cmp expected "$actual"
498498
'
499499

500500
test_expect_success 'prompt - bash color pc mode - dirty status indicator - dirty index' '
501-
printf "BEFORE: (${c_green}master${c_clear} ${c_green}+${c_clear}):AFTER" >expected &&
501+
printf "BEFORE: (${c_green}\${__git_ps1_branch_name}${c_clear} ${c_green}+${c_clear}):AFTER\\nmaster" >expected &&
502502
echo "dirty" >file &&
503503
test_when_finished "git reset --hard" &&
504504
git add -u &&
505505
(
506506
GIT_PS1_SHOWDIRTYSTATE=y &&
507507
GIT_PS1_SHOWCOLORHINTS=y &&
508508
__git_ps1 "BEFORE:" ":AFTER" &&
509-
printf "%s" "$PS1" >"$actual"
509+
printf "%s\\n%s" "$PS1" "${__git_ps1_branch_name}" >"$actual"
510510
) &&
511511
test_cmp expected "$actual"
512512
'
513513

514514
test_expect_success 'prompt - bash color pc mode - dirty status indicator - dirty index and worktree' '
515-
printf "BEFORE: (${c_green}master${c_clear} ${c_red}*${c_green}+${c_clear}):AFTER" >expected &&
515+
printf "BEFORE: (${c_green}\${__git_ps1_branch_name}${c_clear} ${c_red}*${c_green}+${c_clear}):AFTER\\nmaster" >expected &&
516516
echo "dirty index" >file &&
517517
test_when_finished "git reset --hard" &&
518518
git add -u &&
@@ -521,69 +521,69 @@ test_expect_success 'prompt - bash color pc mode - dirty status indicator - dirt
521521
GIT_PS1_SHOWCOLORHINTS=y &&
522522
GIT_PS1_SHOWDIRTYSTATE=y &&
523523
__git_ps1 "BEFORE:" ":AFTER" &&
524-
printf "%s" "$PS1" >"$actual"
524+
printf "%s\\n%s" "$PS1" "${__git_ps1_branch_name}" >"$actual"
525525
) &&
526526
test_cmp expected "$actual"
527527
'
528528

529529
test_expect_success 'prompt - bash color pc mode - dirty status indicator - before root commit' '
530-
printf "BEFORE: (${c_green}master${c_clear} ${c_green}#${c_clear}):AFTER" >expected &&
530+
printf "BEFORE: (${c_green}\${__git_ps1_branch_name}${c_clear} ${c_green}#${c_clear}):AFTER\\nmaster" >expected &&
531531
(
532532
GIT_PS1_SHOWDIRTYSTATE=y &&
533533
GIT_PS1_SHOWCOLORHINTS=y &&
534534
cd otherrepo &&
535535
__git_ps1 "BEFORE:" ":AFTER" &&
536-
printf "%s" "$PS1" >"$actual"
536+
printf "%s\\n%s" "$PS1" "${__git_ps1_branch_name}" >"$actual"
537537
) &&
538538
test_cmp expected "$actual"
539539
'
540540

541541
test_expect_success 'prompt - bash color pc mode - inside .git directory' '
542-
printf "BEFORE: (${c_green}GIT_DIR!${c_clear}):AFTER" >expected &&
542+
printf "BEFORE: (${c_green}\${__git_ps1_branch_name}${c_clear}):AFTER\\nGIT_DIR!" >expected &&
543543
echo "dirty" >file &&
544544
test_when_finished "git reset --hard" &&
545545
(
546546
GIT_PS1_SHOWDIRTYSTATE=y &&
547547
GIT_PS1_SHOWCOLORHINTS=y &&
548548
cd .git &&
549549
__git_ps1 "BEFORE:" ":AFTER" &&
550-
printf "%s" "$PS1" >"$actual"
550+
printf "%s\\n%s" "$PS1" "${__git_ps1_branch_name}" >"$actual"
551551
) &&
552552
test_cmp expected "$actual"
553553
'
554554

555555
test_expect_success 'prompt - bash color pc mode - stash status indicator' '
556-
printf "BEFORE: (${c_green}master${c_clear} ${c_lblue}\$${c_clear}):AFTER" >expected &&
556+
printf "BEFORE: (${c_green}\${__git_ps1_branch_name}${c_clear} ${c_lblue}\$${c_clear}):AFTER\\nmaster" >expected &&
557557
echo 2 >file &&
558558
git stash &&
559559
test_when_finished "git stash drop" &&
560560
(
561561
GIT_PS1_SHOWSTASHSTATE=y &&
562562
GIT_PS1_SHOWCOLORHINTS=y &&
563563
__git_ps1 "BEFORE:" ":AFTER" &&
564-
printf "%s" "$PS1" >"$actual"
564+
printf "%s\\n%s" "$PS1" "${__git_ps1_branch_name}" >"$actual"
565565
) &&
566566
test_cmp expected "$actual"
567567
'
568568

569569
test_expect_success 'prompt - bash color pc mode - untracked files status indicator' '
570-
printf "BEFORE: (${c_green}master${c_clear} ${c_red}%%${c_clear}):AFTER" >expected &&
570+
printf "BEFORE: (${c_green}\${__git_ps1_branch_name}${c_clear} ${c_red}%%${c_clear}):AFTER\\nmaster" >expected &&
571571
(
572572
GIT_PS1_SHOWUNTRACKEDFILES=y &&
573573
GIT_PS1_SHOWCOLORHINTS=y &&
574574
__git_ps1 "BEFORE:" ":AFTER" &&
575-
printf "%s" "$PS1" >"$actual"
575+
printf "%s\\n%s" "$PS1" "${__git_ps1_branch_name}" >"$actual"
576576
) &&
577577
test_cmp expected "$actual"
578578
'
579579

580580
test_expect_success 'prompt - zsh color pc mode' '
581-
printf "BEFORE: (%%F{green}master%%f):AFTER" >expected &&
581+
printf "BEFORE: (%%F{green}\${__git_ps1_branch_name}%%f):AFTER\\nmaster" >expected &&
582582
(
583583
ZSH_VERSION=5.0.0 &&
584584
GIT_PS1_SHOWCOLORHINTS=y &&
585585
__git_ps1 "BEFORE:" ":AFTER" >"$actual"
586-
printf "%s" "$PS1" >"$actual"
586+
printf "%s\\n%s" "$PS1" "${__git_ps1_branch_name}" >"$actual"
587587
) &&
588588
test_cmp expected "$actual"
589589
'

0 commit comments

Comments
 (0)