[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: bug on the processing of internet addresses for /etc/security/access.conf



> greetings,
>
> i've found a bug on pam_access while trying to set a username/host access.
> whenever an ip address is used the search failes to find the entry.

sorry to reply to my own post but seems nobody was replying anyway :-(
(TM)

i have made a cvs diff of the patch and uploaded it to the sourceforge
bugtraq on :

  https://sourceforge.net/tracker/?func=detail&aid=419631&group_id=6663&atid=106663

to make my patch clear, what i did was to add one byte to the hn buffer to
store the trailing dot (that is needed to try a match on ip address rather
than on hostname) and has been checked on the code as shown by :

  } else if (tok[(tok_len = strlen(tok)) - 1] == '.') {

so if the hostname connecting has a working PTR the call on
gethostbyname(string) would make an A request and try to match that
address with what is on the access.conf file (inside tok char*).

using a trailing dot to make this work is a pretty obscure and i think
that adding a module parameter would be nicer, but surelly this could
break other implementations/SPECs i am surelly not aware off.

on the call to snprintf i just added the trailing dot so it could match
with what is found on the access.conf (the ip address including the
"required" trailing dot)

also should be noted that since glibc 2.1 the returned value for
snprintf is what the "expected" number of bytes is and not what the
"actual" number of bytes were written, so the check on the returned
address for >= sizeof(hn) shouldn't be triggered unless there was an
overflow attempt and i guess a warning would be a too (actually not coded
for clarity).

  r = snprintf(hn, sizeof(hn), "%u.%u.%u.%u.",
          (unsigned char)h->h_addr[0], (unsigned char)h->h_addr[1],
          (unsigned char)h->h_addr[2], (unsigned char)h->h_addr[3]);
  if (r < 0 || r >= sizeof(hn))
      return (NO);

some logic also could be made clearer if changing gethostbyname(string)
for gethostbyname2(string, AF_INET) and more work for IPv6 could be added
also, but first i would like to know if my thinking is right or i am just
totally lost? (again trading compatibility and portability with
simplicity)

so, is there anything interesting on this or i am just missing the whole
point?

Carlo

PS. i am CC the pam_access writer and the owner of the code i am changing
as shown on pam_accees.c, so all the interested parties should be at least
notified IMHO.





[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index] []