[libcamera-devel] [PATCH 4/4] libcamera: log: Fallback to getenv on non-gnu systems

[Kieran Bingham]

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

-- 
Regards
--
Kieran