From d66a1ace23c5a9fcedefb2b36f328da4972c5d9e Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Mon, 18 Mar 2019 11:34:22 +0100 Subject: [PATCH] contrib/checkpatch: avoid command injection in checkpatch.pl script The capture variables, $1, etc, are not valid unless the match succeeded, and they're not cleared, either. $ git checkout -B C origin/master && \ echo XXXXX > f.txt && \ git add f.txt && \ git commit -m 'this commit does something()' Branch 'C' set up to track remote branch 'master' from 'origin'. Reset branch 'C' Your branch is up to date with 'origin/master'. sh: -c: line 0: syntax error near unexpected token `(' sh: -c: line 0: `git log --abbrev=12 --pretty=format:"%h ('%s')" -1 does something() 2>/dev/null' >>> VALIDATE "a169a98e14 this commit does something()" (commit message):4: Commit 'does something()' does not seem to exist: > Subject: [PATCH] this commit does something() (commit message):4: Refer to the commit id properly: : > Subject: [PATCH] this commit does something() The patch does not validate. --- contrib/scripts/checkpatch.pl | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/contrib/scripts/checkpatch.pl b/contrib/scripts/checkpatch.pl index c020e64f4b..43ea45e833 100755 --- a/contrib/scripts/checkpatch.pl +++ b/contrib/scripts/checkpatch.pl @@ -101,12 +101,18 @@ sub complain sub check_commit { my $commit = shift; - $commit =~ /^([0-9a-f]{5,})/; - my $commit_id = $1; + my $required = shift; + my $commit_id; my $commit_message; + if ($commit =~ /^([0-9a-f]{5,})$/) { + $commit_id = $1; + } else { + return unless $required; + } + if ($commit_id and not system 'git rev-parse --git-dir >/dev/null 2>/dev/null') { - $commit_message = `git log --abbrev=12 --pretty=format:"%h ('%s')" -1 $commit_id 2>/dev/null`; + $commit_message = `git log --abbrev=12 --pretty=format:"%h ('%s')" -1 "$commit_id" 2>/dev/null`; complain "Commit '$commit_id' does not seem to exist" unless $commit_message; } @@ -141,10 +147,10 @@ if ($is_patch) { $check_line = 1; $line = $_; /^---$/ and $is_commit_message = 0; - /^Fixes: *(.*)/ and check_commit ($1); + /^Fixes: *(.*)/ and check_commit ($1, 1); /This reverts commit/ and next; /cherry picked from/ and next; - /\bcommit (.*)/ and check_commit ($1); + /\bcommit (.*)/ and check_commit ($1, 0); next; } else { # We don't handle these yet