2022-08-29 13:07:26

by CGEL

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

From: xupanda <[email protected]>

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 <[email protected]>
Signed-off-by: xupanda <[email protected]>
Reviewed-by: zhanglin <[email protected]>

---
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


2022-09-04 23:16:55

by Doug Smythies

[permalink] [raw]
Subject: 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

Note: Modifying the address list a little with this reply.

On Mon, Aug 29, 2022 at 5:55 AM <[email protected]> wrote:
>
> From: xupanda <[email protected]>
>
> 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 <[email protected]>
> Signed-off-by: xupanda <[email protected]>
> Reviewed-by: zhanglin <[email protected]>

Acked-by: Doug Smythies <[email protected]>

>
> ---
> 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
>

2022-09-06 12:53:12

by srinivas pandruvada

[permalink] [raw]
Subject: 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

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 <[email protected]> wrote:
> >
> > From: xupanda <[email protected]>
> >
> > 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 <[email protected]>
> > Signed-off-by: xupanda <[email protected]>
> > Reviewed-by: zhanglin <[email protected]>
>
> Acked-by: Doug Smythies <[email protected]>
>
> >
> > ---
> >  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
> >