bugs in pam_env

[Piribillo]

Hello Tomas,

I will surely do that, sorry I didn't think about doing it. 

Antonio

> On 27 Feb 2017, at 09:52, Tomas Mraz <tmraz at redhat.com> wrote:
> 
> Hello Antonio,
> 
> can you please report these as individual issues into the GitHub issue
> tracker for the Linux PAM project?
> 
> The issue 1 is already reported by someone - https://github.com/linux-p
> am/linux-pam/issues/6
> 
> There it is actually questionable whether we should change the behavior
> or the documentation as someone could already depend on the current
> behavior.
> 
> Tomas
> 
>> On Sat, 2017-02-25 at 09:54 +0000, Antonio Capanna wrote:
>> Hi,
>> I found some errors and unexpected/confusing behaviour in pam_env.
>> 
>> 1)
>> The user environment file is not parsed with the correct function,
>> but it is parsed as a configuration file. This means that DEFAULT and
>> OVERRIDE are recognized as special words and that parameter expansion
>> is done.
>> Please see pam_env.c at line 826.
>> The parsing seems to work just because of unexpected behaviour #4
>> below (that is, also in config file the syntax "variable=value" is
>> honored, even if because of an error.)
>> 
>> 2)
>>> From what I understand, setting DEFAULT= deletes a variable if
>>> OVERRIDE is empty or non-existing. But with some configuration
>>> combinations you get inconsistent behaviour:
>> 
>> this_variable_is_removedthis_one_is_removed_too
>> DEFAULT=this_one_is_removed_as_well OVERRIDE=""
>> DEFAULT=also_this_one_is_removed DEFAULT=
>> OVERRIDE=but_this_one_is_added_as_empty_var OVERRIDE= DEFAULT=
>> This happens because in pam_env.c, in function _parse_line,
>> variable quoteflg is not properly handled (more specifically, it is
>> decremented even when it shouldn't).
>> 
>> 3)
>> 
>> I don't know if it is a bug or not, but it's most definitely
>> unexpected.If in the env file I write:
>> export a=1
>> it works. But:
>> export  b=2   # note that there are two spacesexport    c=3 # note
>> that there is a TAB character
>> do not work. The parser in _parse_env_file (pam_env.c:220) does not
>> handle anything different from a single space.
>> 
>> 4)
>> If in conf file I write:
>> variable# or
>> variable DEFAULT=
>> 
>> the expected behaviour is to remove that variable from the
>> environment. But the parser does not check for '=' signs, so if have
>> this in the conf file (NOT in the env file):
>> this_does_not_do_what_expected=yeah# or even this:
>> neither_does_this=ohyeah!     DEFAULT=
>> 
>> the module tries to remove the variable
>> "this_does_not_do_what_expected=yeah" from the environment (as per
>> debug log: remove variable "this_does_not_do_what_expected=yeah"),
>> but it results in adding the variable
>> "this_does_not_do_what_expected" instead, with value = "yeah".
>> Note that this is the reason why the parsing of the user environment
>> file seems to work.
>> 
>> 5)
>> The '#' char is recognized as comment even if it's not the first non-
>> blank character in the line (differently from what written in the man
>> page).Especially, a conf line like this one:
>> variable DEFAULT="a#b"
>> does not work, it results in an error (Unterminated quoted string:
>> "a). If not supported, this should be at least mentioned in the
>> manual.Note that the same problem is in the env file, because the
>> line parsing function is the same (_assemble_line).
>> 
>> 6)
>> pam_env.c:74:/* This is a flag used to designate an empty string
>> */static char quote='Z';
>> but in function check_var (row 511):    if (&quote == var->defval) { 
>>     /*       * This means that the empty string was given for defval
>> value       * which indicates that a variable should be defined with
>> no value       */      *var->defval = '\0';
>> as it can be seen, the last line changes the value of quote. This
>> does not seem to have ill effects, but it is not nice because quote
>> becomes = '\0', which has also a special meaning different from quote
>> itself (quote means the empty quoted string, '\0' means the empty
>> unquoted string).
>> 
>> 7)
>> the "_parse_env_file" function in pam_env.c gets lines using function
>> "_assemble_line", but then repeats a lot of the checks that have
>> already been done (comments, empty lines, whitespaces at the
>> beginning, ...)
>> 
>> Regards
>> _______________________________________________
>> Pam-list mailing list
>> Pam-list at redhat.com
>> https://www.redhat.com/mailman/listinfo/pam-list
> -- 
> Tomas Mraz
> No matter how far down the wrong road you've gone, turn back.
>                                              Turkish proverb
> (You'll never know whether the road is wrong though.)
> 
> _______________________________________________
> Pam-list mailing list
> Pam-list at redhat.com
> https://www.redhat.com/mailman/listinfo/pam-list