[PATCH linux-next] tools: power: x86: intel_pstate_tracer: intel_pstate_tracer: Replace the relative path of the called grep with an absolute path

cgel.zte@gmail.com posted 1 patch 3 years, 7 months ago
tools/power/x86/intel_pstate_tracer/intel_pstate_tracer.py | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
[PATCH linux-next] tools: power: x86: intel_pstate_tracer: intel_pstate_tracer: Replace the relative path of the called grep with an absolute path
Posted by cgel.zte@gmail.com 3 years, 7 months ago
From: xupanda <xu.panda@zte.com.cn>

Using absolute paths when invoking grep can lead to serious security
issues. A malicious operator may create a new file named grep in the
PATH before the real grep file. When the program executes and calls
grep, the fake grep file will be executed, so that the malicious
operator can execute arbitrary code or even obtain root privileges.
Using absolute paths when calling grep avoids this problem.

Reported-by: Zeal Robot <zealci@zte.com.cn>
Signed-off-by: xupanda <xu.panda@zte.com.cn>
Reviewed-by: zhanglin <zhang.lin16@zte.com.cn>

---
 tools/power/x86/intel_pstate_tracer/intel_pstate_tracer.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tools/power/x86/intel_pstate_tracer/intel_pstate_tracer.py b/tools/power/x86/intel_pstate_tracer/intel_pstate_tracer.py
index b46e9eb8f5aa..d6fae6e51d30 100755
--- a/tools/power/x86/intel_pstate_tracer/intel_pstate_tracer.py
+++ b/tools/power/x86/intel_pstate_tracer/intel_pstate_tracer.py
@@ -348,8 +348,8 @@ def split_csv(current_max_cpu, cpu_mask):
     if os.path.exists('cpu.csv'):
         for index in range(0, current_max_cpu + 1):
             if cpu_mask[int(index)] != 0:
-                os.system('grep -m 1 common_cpu cpu.csv > cpu{:0>3}.csv'.format(index))
-                os.system('grep CPU_{:0>3} cpu.csv >> cpu{:0>3}.csv'.format(index, index))
+                os.system('/usr/bin/grep -m 1 common_cpu cpu.csv > cpu{:0>3}.csv'.format(index))
+                os.system('/usr/bin/grep CPU_{:0>3} cpu.csv >> cpu{:0>3}.csv'.format(index, index))
 
 def fix_ownership(path):
     """Change the owner of the file to SUDO_UID, if required"""
-- 
2.25.1
Re: [PATCH linux-next] tools: power: x86: intel_pstate_tracer: intel_pstate_tracer: Replace the relative path of the called grep with an absolute path
Posted by Doug Smythies 3 years, 7 months ago
Note: Modifying the address list a little with this reply.

On Mon, Aug 29, 2022 at 5:55 AM <cgel.zte@gmail.com> wrote:
>
> From: xupanda <xu.panda@zte.com.cn>
>
> Using absolute paths when invoking grep can lead to serious security
> issues. A malicious operator may create a new file named grep in the
> PATH before the real grep file. When the program executes and calls
> grep, the fake grep file will be executed, so that the malicious
> operator can execute arbitrary code or even obtain root privileges.
> Using absolute paths when calling grep avoids this problem.
>
> Reported-by: Zeal Robot <zealci@zte.com.cn>
> Signed-off-by: xupanda <xu.panda@zte.com.cn>
> Reviewed-by: zhanglin <zhang.lin16@zte.com.cn>

Acked-by: Doug Smythies <dsmythies@telus.net>

>
> ---
>  tools/power/x86/intel_pstate_tracer/intel_pstate_tracer.py | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/tools/power/x86/intel_pstate_tracer/intel_pstate_tracer.py b/tools/power/x86/intel_pstate_tracer/intel_pstate_tracer.py
> index b46e9eb8f5aa..d6fae6e51d30 100755
> --- a/tools/power/x86/intel_pstate_tracer/intel_pstate_tracer.py
> +++ b/tools/power/x86/intel_pstate_tracer/intel_pstate_tracer.py
> @@ -348,8 +348,8 @@ def split_csv(current_max_cpu, cpu_mask):
>      if os.path.exists('cpu.csv'):
>          for index in range(0, current_max_cpu + 1):
>              if cpu_mask[int(index)] != 0:
> -                os.system('grep -m 1 common_cpu cpu.csv > cpu{:0>3}.csv'.format(index))
> -                os.system('grep CPU_{:0>3} cpu.csv >> cpu{:0>3}.csv'.format(index, index))
> +                os.system('/usr/bin/grep -m 1 common_cpu cpu.csv > cpu{:0>3}.csv'.format(index))
> +                os.system('/usr/bin/grep CPU_{:0>3} cpu.csv >> cpu{:0>3}.csv'.format(index, index))
>
>  def fix_ownership(path):
>      """Change the owner of the file to SUDO_UID, if required"""
> --
> 2.25.1
>
Re: [PATCH linux-next] tools: power: x86: intel_pstate_tracer: intel_pstate_tracer: Replace the relative path of the called grep with an absolute path
Posted by srinivas pandruvada 3 years, 7 months ago
On Sun, 2022-09-04 at 15:23 -0700, Doug Smythies wrote:
> Note: Modifying the address list a little with this reply.
> 

Subject can be "Use absolute path for grep"

> On Mon, Aug 29, 2022 at 5:55 AM <cgel.zte@gmail.com> wrote:
> > 
> > From: xupanda <xu.panda@zte.com.cn>
> > 
> > Using absolute paths when invoking grep can lead to serious
> > security
> > issues. 
I think you mean 
"Not using absolute path when invoking grep can lead to serious
security issues"

Thanks,
Srinivas

> > A malicious operator may create a new file named grep in the
> > PATH before the real grep file. When the program executes and calls
> > grep, the fake grep file will be executed, so that the malicious
> > operator can execute arbitrary code or even obtain root privileges.
> > Using absolute paths when calling grep avoids this problem.
> > 
> > Reported-by: Zeal Robot <zealci@zte.com.cn>
> > Signed-off-by: xupanda <xu.panda@zte.com.cn>
> > Reviewed-by: zhanglin <zhang.lin16@zte.com.cn>
> 
> Acked-by: Doug Smythies <dsmythies@telus.net>
> 
> > 
> > ---
> >  tools/power/x86/intel_pstate_tracer/intel_pstate_tracer.py | 4 ++-
> > -
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git
> > a/tools/power/x86/intel_pstate_tracer/intel_pstate_tracer.py
> > b/tools/power/x86/intel_pstate_tracer/intel_pstate_tracer.py
> > index b46e9eb8f5aa..d6fae6e51d30 100755
> > --- a/tools/power/x86/intel_pstate_tracer/intel_pstate_tracer.py
> > +++ b/tools/power/x86/intel_pstate_tracer/intel_pstate_tracer.py
> > @@ -348,8 +348,8 @@ def split_csv(current_max_cpu, cpu_mask):
> >      if os.path.exists('cpu.csv'):
> >          for index in range(0, current_max_cpu + 1):
> >              if cpu_mask[int(index)] != 0:
> > -                os.system('grep -m 1 common_cpu cpu.csv >
> > cpu{:0>3}.csv'.format(index))
> > -                os.system('grep CPU_{:0>3} cpu.csv >>
> > cpu{:0>3}.csv'.format(index, index))
> > +                os.system('/usr/bin/grep -m 1 common_cpu cpu.csv >
> > cpu{:0>3}.csv'.format(index))
> > +                os.system('/usr/bin/grep CPU_{:0>3} cpu.csv >>
> > cpu{:0>3}.csv'.format(index, index))
> > 
> >  def fix_ownership(path):
> >      """Change the owner of the file to SUDO_UID, if required"""
> > --
> > 2.25.1
> >