| Message ID | 20190322104350.31091-5-kieran.bingham@ideasonboard.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series |
|
| Related | show |
Hi Kieran, Thank you for the patch. On Fri, Mar 22, 2019 at 10:43:50AM +0000, Kieran Bingham wrote: > The secure_getenv() call is not provided by all toolchains. Support > compilation without this feature by falling back to the default getenv() > functionality. > > Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > --- > src/libcamera/log.cpp | 9 +++++++++ > 1 file changed, 9 insertions(+) > > diff --git a/src/libcamera/log.cpp b/src/libcamera/log.cpp > index 7d930cd6b99e..a44bd941e615 100644 > --- a/src/libcamera/log.cpp > +++ b/src/libcamera/log.cpp > @@ -49,6 +49,15 @@ > > namespace libcamera { > > + > +/* > + * secure_getenv is a GNU-specific extension to the C-Library. > + * fall back to the default getenv when it is not available. > + */ > +#ifndef HAVE_SECURE_GETENV Where is HAVE_SECURE_GETENV defined ? > +#define secure_getenv getenv This means that a setuid-root binary linked to libcamera could be used to overwrite any file on the system through the logging infrastructure. There should be no setuid binaries linking to libcamera in the first place, but I would still prefer avoiding this potential security issue. > +#endif > + > /** > * \brief Message logger > *
Hi Laurent, On 22/03/2019 23:03, Laurent Pinchart wrote: > Hi Kieran, > > Thank you for the patch. > > On Fri, Mar 22, 2019 at 10:43:50AM +0000, Kieran Bingham wrote: >> The secure_getenv() call is not provided by all toolchains. Support >> compilation without this feature by falling back to the default getenv() >> functionality. >> >> Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com> >> --- >> src/libcamera/log.cpp | 9 +++++++++ >> 1 file changed, 9 insertions(+) >> >> diff --git a/src/libcamera/log.cpp b/src/libcamera/log.cpp >> index 7d930cd6b99e..a44bd941e615 100644 >> --- a/src/libcamera/log.cpp >> +++ b/src/libcamera/log.cpp >> @@ -49,6 +49,15 @@ >> >> namespace libcamera { >> >> + >> +/* >> + * secure_getenv is a GNU-specific extension to the C-Library. >> + * fall back to the default getenv when it is not available. >> + */ >> +#ifndef HAVE_SECURE_GETENV > > Where is HAVE_SECURE_GETENV defined ? Ah - I thought it was provided by glibc - but I think I was mistaken. We'll have to do some checking in meson.build: if compiler.has_function('secure_getenv', prefix : '#include <stdlib.h>') # - add -DHAVE_SECURE_GETENV to flags or such... endif >> +#define secure_getenv getenv > > This means that a setuid-root binary linked to libcamera could be used > to overwrite any file on the system through the logging infrastructure. > There should be no setuid binaries linking to libcamera in the first > place, but I would still prefer avoiding this potential security issue. Sure, I get that it's useful - but it's not always available ... > https://www.gnu.org/software/libc/manual/html_node/Environment-Access.html: > > Function: char * secure_getenv (const char *name) > > Preliminary: | MT-Safe env | AS-Safe | AC-Safe | See POSIX Safety Concepts. > > This function is similar to getenv, but it returns a null pointer if the environment is untrusted. This happens when the program file has SUID or SGID bits set. General-purpose libraries should always prefer this function over getenv to avoid vulnerabilities if the library is referenced from a SUID/SGID program. > > This function is a GNU extension. ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ is the issue. >> +#endif >> + >> /** >> * \brief Message logger >> * >
diff --git a/src/libcamera/log.cpp b/src/libcamera/log.cpp index 7d930cd6b99e..a44bd941e615 100644 --- a/src/libcamera/log.cpp +++ b/src/libcamera/log.cpp @@ -49,6 +49,15 @@ namespace libcamera { + +/* + * secure_getenv is a GNU-specific extension to the C-Library. + * fall back to the default getenv when it is not available. + */ +#ifndef HAVE_SECURE_GETENV +#define secure_getenv getenv +#endif + /** * \brief Message logger *
The secure_getenv() call is not provided by all toolchains. Support compilation without this feature by falling back to the default getenv() functionality. Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com> --- src/libcamera/log.cpp | 9 +++++++++ 1 file changed, 9 insertions(+)