Bug 1498210

Has already been fixed by bug 1455923 which included me fixing a bunch of signedness issues

*** This bug has been marked as a duplicate of bug 1455923 ***

(In reply to Justin Mitchell from comment #7)
> Has already been fixed by bug 1455923 which included me fixing a bunch of
> signedness issues

Re-thinking... is there a single patch that just addresses this issue?

The closest committed patch would be 545b7409 in the nfs-utils tree, but thats after libnfsidmap was merged in.

On its own the relevant chunk that could be applied to the libnfsidmap tree would be:

diff --git a/libnfsidmap.c b/libnfsidmap.c
--- a/libnfsidmap.c
+++ b/libnfsidmap.c
@@ -128,7 +128,8 @@ static int id_as_chars(char *name, uid_t *id)
 static int dns_txt_query(char *domain, char **nfs4domain)
 {
        char *txtname = NFS4DNSTXTREC;
-       char *msg, *answ, *eom, *mptr; 
+       unsigned char *msg, *eom, *mptr;
+       char *answ;
        int len, status = -1;
        HEADER *hdr;

(In reply to Justin Mitchell from comment #10)
> The closest committed patch would be 545b7409 in the nfs-utils tree, but
> thats after libnfsidmap was merged in.
> 
> On its own the relevant chunk that could be applied to the libnfsidmap tree
> would be:
> 
> diff --git a/libnfsidmap.c b/libnfsidmap.c
> --- a/libnfsidmap.c
> +++ b/libnfsidmap.c
> @@ -128,7 +128,8 @@ static int id_as_chars(char *name, uid_t *id)
>  static int dns_txt_query(char *domain, char **nfs4domain)
>  {
>         char *txtname = NFS4DNSTXTREC;
> -       char *msg, *answ, *eom, *mptr; 
> +       unsigned char *msg, *eom, *mptr;
> +       char *answ;
>         int len, status = -1;
>         HEADER *hdr;

This seems simple enough with little risk... 

Devel Ack

Yes, I believe this would also work, since the length would then be cast from (unsigned char) to (int), and the length should stay a positive number:

...
       unsigned char *msg, *eom, *mptr;
       int len, status = -1;
...
       /* get the lenght field */
       len = (int)*mptr++;
       /* copy the data */
       memcpy(answ, mptr, len);
       answ[len] = '\0';

as far as I can tell, it should be safe for 'answ' to remain signed, though I haven't looked closely.

I had already made these changes when merging libnfsidmap info nfs-utils as i was trying to clear the many warnings the old code produced, then wondered why i couldn't reproduce this bug when using that code base.

How likely or important is it to support hostnames longer then 255 chars ? as if needed i can work a proper fix into a future patch.

(In reply to Steve Dickson from comment #11)
> (In reply to Justin Mitchell from comment #10)
> > The closest committed patch would be 545b7409 in the nfs-utils tree, but
> > thats after libnfsidmap was merged in.
> > 
> > On its own the relevant chunk that could be applied to the libnfsidmap tree
> > would be:
> > 
> > diff --git a/libnfsidmap.c b/libnfsidmap.c
> > --- a/libnfsidmap.c
> > +++ b/libnfsidmap.c
> > @@ -128,7 +128,8 @@ static int id_as_chars(char *name, uid_t *id)
> >  static int dns_txt_query(char *domain, char **nfs4domain)
> >  {
> >         char *txtname = NFS4DNSTXTREC;
> > -       char *msg, *answ, *eom, *mptr; 
> > +       unsigned char *msg, *eom, *mptr;
> > +       char *answ;
> >         int len, status = -1;
> >         HEADER *hdr;
> 
> This seems simple enough with little risk... 
> 
> Devel Ack

Is there a similar patch we can do for bug 1455923?

1455923 is not so easy, there i completely rewrote the line parser as it was just terrible code that overran its buffer, so its difficult to fix just the one issue. I will work up a patch with a more minimal rewite of the parser for use if we cant manage to rebase to the newer tree.

This problem has been fixed in libnfsidmap-0.25-18.el7

Moving to VERIFIED according to Comment #17.

Since the problem described in this bug report should be
resolved in a recent advisory, it has been closed with a
resolution of ERRATA.

For information on the advisory, and where to find the updated
files, follow the link below.

If the solution does not work for you, open a new bug report.

https://access.redhat.com/errata/RHBA-2018:1016