Skip to content

Commit 6eca9c0

Browse files
gitstergitster
committed
Merge branch 'rh/prompt-pcmode-avoid-eval-on-refname' into maint
The shell prompt script (in contrib/), when using the PROMPT_COMMAND interface, used an unsafe construct when showing the branch name in $PS1. * rh/prompt-pcmode-avoid-eval-on-refname: git-prompt.sh: don't put unsanitized branch names in $PS1
2 parents e79fcfc + 8976500 commit 6eca9c0

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)