[PATCH] ebuild.sh: Completely ban external commands in global scope

classic Classic list List threaded Threaded
7 messages Options
Reply | Threaded
Open this post in threaded view
|

[PATCH] ebuild.sh: Completely ban external commands in global scope

Michał Górny-5
Set PATH to /dev/null when sourcing the ebuild for dependency resolution
in order to prevent shell from finding external commands via PATH
lookup. While this does not prevent executing programs via full path, it
should catch the majority of accidental uses.

Closes: https://github.com/gentoo/portage/pull/199

// Note: this can't be merged right now since we still have ebuilds
// calling external commands; see:
// https://bugs.gentoo.org/show_bug.cgi?id=629222
---
 bin/ebuild.sh             | 6 +++++-
 bin/isolated-functions.sh | 4 ++++
 2 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/bin/ebuild.sh b/bin/ebuild.sh
index c23561651..94a44d534 100755
--- a/bin/ebuild.sh
+++ b/bin/ebuild.sh
@@ -80,8 +80,12 @@ else
  done
  unset funcs x
 
+ # prevent the shell from finding external executables
+ # note: we can't use empty because it implies current directory
+ _PORTAGE_ORIG_PATH=${PATH}
+ export PATH=/dev/null
  command_not_found_handle() {
- die "Command not found while sourcing ebuild: ${*}"
+ die "External commands disallowed while sourcing ebuild: ${*}"
  }
 fi
 
diff --git a/bin/isolated-functions.sh b/bin/isolated-functions.sh
index e320f7132..b28e44f18 100644
--- a/bin/isolated-functions.sh
+++ b/bin/isolated-functions.sh
@@ -121,6 +121,10 @@ __helpers_die() {
 }
 
 die() {
+ # restore PATH since die calls basename & sed
+ # TODO: make it pure bash
+ [[ -n ${_PORTAGE_ORIG_PATH} ]] && PATH=${_PORTAGE_ORIG_PATH}
+
  set +x # tracing only produces useless noise here
  local IFS=$' \t\n'
 
--
2.14.1


Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] ebuild.sh: Completely ban external commands in global scope

Michał Górny-5
Dnia 31 sierpnia 2017 22:45:42 CEST, "Michał Górny" <[hidden email]> napisał(a):

>Set PATH to /dev/null when sourcing the ebuild for dependency
>resolution
>in order to prevent shell from finding external commands via PATH
>lookup. While this does not prevent executing programs via full path,
>it
>should catch the majority of accidental uses.
>
>Closes: https://github.com/gentoo/portage/pull/199
>
>// Note: this can't be merged right now since we still have ebuilds
>// calling external commands; see:
>// https://bugs.gentoo.org/show_bug.cgi?id=629222

Update: gentoo is green now

>---
> bin/ebuild.sh             | 6 +++++-
> bin/isolated-functions.sh | 4 ++++
> 2 files changed, 9 insertions(+), 1 deletion(-)
>
>diff --git a/bin/ebuild.sh b/bin/ebuild.sh
>index c23561651..94a44d534 100755
>--- a/bin/ebuild.sh
>+++ b/bin/ebuild.sh
>@@ -80,8 +80,12 @@ else
> done
> unset funcs x
>
>+ # prevent the shell from finding external executables
>+ # note: we can't use empty because it implies current directory
>+ _PORTAGE_ORIG_PATH=${PATH}
>+ export PATH=/dev/null
> command_not_found_handle() {
>- die "Command not found while sourcing ebuild: ${*}"
>+ die "External commands disallowed while sourcing ebuild: ${*}"
> }
> fi
>
>diff --git a/bin/isolated-functions.sh b/bin/isolated-functions.sh
>index e320f7132..b28e44f18 100644
>--- a/bin/isolated-functions.sh
>+++ b/bin/isolated-functions.sh
>@@ -121,6 +121,10 @@ __helpers_die() {
> }
>
> die() {
>+ # restore PATH since die calls basename & sed
>+ # TODO: make it pure bash
>+ [[ -n ${_PORTAGE_ORIG_PATH} ]] && PATH=${_PORTAGE_ORIG_PATH}
>+
> set +x # tracing only produces useless noise here
> local IFS=$' \t\n'
>


--
Best regards,
Michał Górny (by phone)

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] ebuild.sh: Completely ban external commands in global scope

Alec Warner-2
Why PATH=/dev/null vs export PATH=""

On Thu, Sep 7, 2017 at 3:36 AM, Michał Górny <[hidden email]> wrote:
Dnia 31 sierpnia 2017 22:45:42 CEST, "Michał Górny" <[hidden email]> napisał(a):
>Set PATH to /dev/null when sourcing the ebuild for dependency
>resolution
>in order to prevent shell from finding external commands via PATH
>lookup. While this does not prevent executing programs via full path,
>it
>should catch the majority of accidental uses.
>
>Closes: https://github.com/gentoo/portage/pull/199
>
>// Note: this can't be merged right now since we still have ebuilds
>// calling external commands; see:
>// https://bugs.gentoo.org/show_bug.cgi?id=629222

Update: gentoo is green now

>---
> bin/ebuild.sh             | 6 +++++-
> bin/isolated-functions.sh | 4 ++++
> 2 files changed, 9 insertions(+), 1 deletion(-)
>
>diff --git a/bin/ebuild.sh b/bin/ebuild.sh
>index c23561651..94a44d534 100755
>--- a/bin/ebuild.sh
>+++ b/bin/ebuild.sh
>@@ -80,8 +80,12 @@ else
>       done
>       unset funcs x
>
>+      # prevent the shell from finding external executables
>+      # note: we can't use empty because it implies current directory
>+      _PORTAGE_ORIG_PATH=${PATH}
>+      export PATH=/dev/null
>       command_not_found_handle() {
>-              die "Command not found while sourcing ebuild: ${*}"
>+              die "External commands disallowed while sourcing ebuild: ${*}"
>       }
> fi
>
>diff --git a/bin/isolated-functions.sh b/bin/isolated-functions.sh
>index e320f7132..b28e44f18 100644
>--- a/bin/isolated-functions.sh
>+++ b/bin/isolated-functions.sh
>@@ -121,6 +121,10 @@ __helpers_die() {
> }
>
> die() {
>+      # restore PATH since die calls basename & sed
>+      # TODO: make it pure bash
>+      [[ -n ${_PORTAGE_ORIG_PATH} ]] && PATH=${_PORTAGE_ORIG_PATH}
>+
>       set +x # tracing only produces useless noise here
>       local IFS=$' \t\n'
>


--
Best regards,
Michał Górny (by phone)


Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] ebuild.sh: Completely ban external commands in global scope

Michał Górny-5
W dniu pią, 08.09.2017 o godzinie 14∶48 -0400, użytkownik Alec Warner
napisał:
> Why PATH=/dev/null vs export PATH=""

+      # note: we can't use empty because it implies current directory

>
> On Thu, Sep 7, 2017 at 3:36 AM, Michał Górny <[hidden email]> wrote:
>
> > Dnia 31 sierpnia 2017 22:45:42 CEST, "Michał Górny" <[hidden email]>
> > napisał(a):
> > > Set PATH to /dev/null when sourcing the ebuild for dependency
> > > resolution
> > > in order to prevent shell from finding external commands via PATH
> > > lookup. While this does not prevent executing programs via full path,
> > > it
> > > should catch the majority of accidental uses.
> > >
> > > Closes: https://github.com/gentoo/portage/pull/199
> > >
> > > // Note: this can't be merged right now since we still have ebuilds
> > > // calling external commands; see:
> > > // https://bugs.gentoo.org/show_bug.cgi?id=629222
> >
> > Update: gentoo is green now
> >
> > > ---
> > > bin/ebuild.sh             | 6 +++++-
> > > bin/isolated-functions.sh | 4 ++++
> > > 2 files changed, 9 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/bin/ebuild.sh b/bin/ebuild.sh
> > > index c23561651..94a44d534 100755
> > > --- a/bin/ebuild.sh
> > > +++ b/bin/ebuild.sh
> > > @@ -80,8 +80,12 @@ else
> > >       done
> > >       unset funcs x
> > >
> > > +      # prevent the shell from finding external executables
> > > +      # note: we can't use empty because it implies current directory
> > > +      _PORTAGE_ORIG_PATH=${PATH}
> > > +      export PATH=/dev/null
> > >       command_not_found_handle() {
> > > -              die "Command not found while sourcing ebuild: ${*}"
> > > +              die "External commands disallowed while sourcing ebuild:
> >
> > ${*}"
> > >       }
> > > fi
> > >
> > > diff --git a/bin/isolated-functions.sh b/bin/isolated-functions.sh
> > > index e320f7132..b28e44f18 100644
> > > --- a/bin/isolated-functions.sh
> > > +++ b/bin/isolated-functions.sh
> > > @@ -121,6 +121,10 @@ __helpers_die() {
> > > }
> > >
> > > die() {
> > > +      # restore PATH since die calls basename & sed
> > > +      # TODO: make it pure bash
> > > +      [[ -n ${_PORTAGE_ORIG_PATH} ]] && PATH=${_PORTAGE_ORIG_PATH}
> > > +
> > >       set +x # tracing only produces useless noise here
> > >       local IFS=$' \t\n'
> > >
> >
> >
> > --
> > Best regards,
> > Michał Górny (by phone)
> >
> >

--
Best regards,
Michał Górny


Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] ebuild.sh: Completely ban external commands in global scope

Alec Warner-2
Must be old age setting in :(

Thanks,

-A

On Fri, Sep 8, 2017 at 2:54 PM, Michał Górny <[hidden email]> wrote:
W dniu pią, 08.09.2017 o godzinie 14∶48 -0400, użytkownik Alec Warner
napisał:
> Why PATH=/dev/null vs export PATH=""

+      # note: we can't use empty because it implies current directory

>
> On Thu, Sep 7, 2017 at 3:36 AM, Michał Górny <[hidden email]> wrote:
>
> > Dnia 31 sierpnia 2017 22:45:42 CEST, "Michał Górny" <[hidden email]>
> > napisał(a):
> > > Set PATH to /dev/null when sourcing the ebuild for dependency
> > > resolution
> > > in order to prevent shell from finding external commands via PATH
> > > lookup. While this does not prevent executing programs via full path,
> > > it
> > > should catch the majority of accidental uses.
> > >
> > > Closes: https://github.com/gentoo/portage/pull/199
> > >
> > > // Note: this can't be merged right now since we still have ebuilds
> > > // calling external commands; see:
> > > // https://bugs.gentoo.org/show_bug.cgi?id=629222
> >
> > Update: gentoo is green now
> >
> > > ---
> > > bin/ebuild.sh             | 6 +++++-
> > > bin/isolated-functions.sh | 4 ++++
> > > 2 files changed, 9 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/bin/ebuild.sh b/bin/ebuild.sh
> > > index c23561651..94a44d534 100755
> > > --- a/bin/ebuild.sh
> > > +++ b/bin/ebuild.sh
> > > @@ -80,8 +80,12 @@ else
> > >       done
> > >       unset funcs x
> > >
> > > +      # prevent the shell from finding external executables
> > > +      # note: we can't use empty because it implies current directory
> > > +      _PORTAGE_ORIG_PATH=${PATH}
> > > +      export PATH=/dev/null
> > >       command_not_found_handle() {
> > > -              die "Command not found while sourcing ebuild: ${*}"
> > > +              die "External commands disallowed while sourcing ebuild:
> >
> > ${*}"
> > >       }
> > > fi
> > >
> > > diff --git a/bin/isolated-functions.sh b/bin/isolated-functions.sh
> > > index e320f7132..b28e44f18 100644
> > > --- a/bin/isolated-functions.sh
> > > +++ b/bin/isolated-functions.sh
> > > @@ -121,6 +121,10 @@ __helpers_die() {
> > > }
> > >
> > > die() {
> > > +      # restore PATH since die calls basename & sed
> > > +      # TODO: make it pure bash
> > > +      [[ -n ${_PORTAGE_ORIG_PATH} ]] && PATH=${_PORTAGE_ORIG_PATH}
> > > +
> > >       set +x # tracing only produces useless noise here
> > >       local IFS=$' \t\n'
> > >
> >
> >
> > --
> > Best regards,
> > Michał Górny (by phone)
> >
> >

--
Best regards,
Michał Górny



Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] ebuild.sh: Completely ban external commands in global scope

Robin H. Johnson-2
In reply to this post by Michał Górny-5
On Thu, Aug 31, 2017 at 10:45:42PM +0200, Michał Górny wrote:
> + export PATH=/dev/null
Minor nitpick: The Single UNIX spec says that PATH is a set of prefixes,
and that they're treated as directories.
http://pubs.opengroup.org/onlinepubs/7908799/xbd/envvar.html

I think it might be good to use either a non-existing path, or a known
empty directory (/var/empty), rather than /dev/null which DOES exist.

--
Robin Hugh Johnson
Gentoo Linux: Dev, Infra Lead, Foundation Asst. Treasurer
E-Mail   : [hidden email]
GnuPG FP : 11ACBA4F 4778E3F6 E4EDF38E B27B944E 34884E85
GnuPG FP : 7D0B3CEB E9B85B1F 825BCECF EE05E6F6 A48F6136

signature.asc (1K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] ebuild.sh: Completely ban external commands in global scope

Ulrich Mueller-2
>>>>> On Fri, 8 Sep 2017, Robin H Johnson wrote:

> On Thu, Aug 31, 2017 at 10:45:42PM +0200, Michał Górny wrote:
>> + export PATH=/dev/null
> Minor nitpick: The Single UNIX spec says that PATH is a set of
> prefixes, and that they're treated as directories.
> http://pubs.opengroup.org/onlinepubs/7908799/xbd/envvar.html

> I think it might be good to use either a non-existing path, or a
> known empty directory (/var/empty), rather than /dev/null which DOES
> exist.

Is /var/empty standard? On my system here, it belongs to
net-misc/openssh.

Also any /dev/null/foo is guaranteed not to exist, so I don't see how
pathname resolution could possibly succeed when PATH is /dev/null.

Ulrich

attachment0 (501 bytes) Download Attachment