Message ID | 87a7luh87n.fsf@oldenburg.str.redhat.com |
---|---|
State | New |
Headers | show |
Series | CVE-2018-19591: if_nametoindex: Fix descriptor for overlong name [BZ #23927] | expand |
On Nov 27 2018, Florian Weimer <fweimer@redhat.com> wrote: > diff --git a/sysdeps/unix/sysv/linux/if_index.c b/sysdeps/unix/sysv/linux/if_index.c > index e3d08982d9..3e999d6509 100644 > --- a/sysdeps/unix/sysv/linux/if_index.c > +++ b/sysdeps/unix/sysv/linux/if_index.c > @@ -45,6 +45,7 @@ __if_nametoindex (const char *ifname) > > if (strlen (ifname) >= IFNAMSIZ) > { > + __close_nocancel_nostatus (fd); > __set_errno (ENODEV); > return 0; > } Just move it before the opensock call. Andreas.
* Andreas Schwab: > On Nov 27 2018, Florian Weimer <fweimer@redhat.com> wrote: > >> diff --git a/sysdeps/unix/sysv/linux/if_index.c b/sysdeps/unix/sysv/linux/if_index.c >> index e3d08982d9..3e999d6509 100644 >> --- a/sysdeps/unix/sysv/linux/if_index.c >> +++ b/sysdeps/unix/sysv/linux/if_index.c >> @@ -45,6 +45,7 @@ __if_nametoindex (const char *ifname) >> >> if (strlen (ifname) >= IFNAMSIZ) >> { >> + __close_nocancel_nostatus (fd); >> __set_errno (ENODEV); >> return 0; >> } > > Just move it before the opensock call. Like this? 2018-11-27 Florian Weimer <fweimer@redhat.com> [BZ #23927] CVE-2018-19591 * sysdeps/unix/sysv/linux/if_index.c (__if_nametoindex): Avoid descriptor leak in case of ENODEV error. diff --git a/NEWS b/NEWS index f488821af1..1098be1afb 100644 --- a/NEWS +++ b/NEWS @@ -57,7 +57,9 @@ Changes to build and runtime requirements: Security related changes: - [Add security related changes here] + CVE-2018-19591: A file descriptor leak in if_nametoindex can lead to a + denial of service due to resource exhaustion when processing getaddrinfo + calls with crafted host names. Reported by Guido Vranken. The following bugs are resolved with this release: diff --git a/sysdeps/unix/sysv/linux/if_index.c b/sysdeps/unix/sysv/linux/if_index.c index e3d08982d9..782fc5e175 100644 --- a/sysdeps/unix/sysv/linux/if_index.c +++ b/sysdeps/unix/sysv/linux/if_index.c @@ -38,11 +38,6 @@ __if_nametoindex (const char *ifname) return 0; #else struct ifreq ifr; - int fd = __opensock (); - - if (fd < 0) - return 0; - if (strlen (ifname) >= IFNAMSIZ) { __set_errno (ENODEV); @@ -50,6 +45,12 @@ __if_nametoindex (const char *ifname) } strncpy (ifr.ifr_name, ifname, sizeof (ifr.ifr_name)); + + int fd = __opensock (); + + if (fd < 0) + return 0; + if (__ioctl (fd, SIOCGIFINDEX, &ifr) < 0) { int saved_errno = errno;
Ok. Andreas.
On Tue, Nov 27, 2018 at 9:13 AM Florian Weimer <fweimer@redhat.com> wrote: > > * Andreas Schwab: > > > On Nov 27 2018, Florian Weimer <fweimer@redhat.com> wrote: > > > >> diff --git a/sysdeps/unix/sysv/linux/if_index.c b/sysdeps/unix/sysv/linux/if_index.c > >> index e3d08982d9..3e999d6509 100644 > >> --- a/sysdeps/unix/sysv/linux/if_index.c > >> +++ b/sysdeps/unix/sysv/linux/if_index.c > >> @@ -45,6 +45,7 @@ __if_nametoindex (const char *ifname) > >> > >> if (strlen (ifname) >= IFNAMSIZ) > >> { > >> + __close_nocancel_nostatus (fd); > >> __set_errno (ENODEV); > >> return 0; > >> } > > > > Just move it before the opensock call. > > Like this? > > 2018-11-27 Florian Weimer <fweimer@redhat.com> > > [BZ #23927] > CVE-2018-19591 > * sysdeps/unix/sysv/linux/if_index.c (__if_nametoindex): Avoid > descriptor leak in case of ENODEV error. > > diff --git a/NEWS b/NEWS > index f488821af1..1098be1afb 100644 > --- a/NEWS > +++ b/NEWS > @@ -57,7 +57,9 @@ Changes to build and runtime requirements: > > Security related changes: > > - [Add security related changes here] > + CVE-2018-19591: A file descriptor leak in if_nametoindex can lead to a > + denial of service due to resource exhaustion when processing getaddrinfo > + calls with crafted host names. Reported by Guido Vranken. > > The following bugs are resolved with this release: > > diff --git a/sysdeps/unix/sysv/linux/if_index.c b/sysdeps/unix/sysv/linux/if_index.c > index e3d08982d9..782fc5e175 100644 > --- a/sysdeps/unix/sysv/linux/if_index.c > +++ b/sysdeps/unix/sysv/linux/if_index.c > @@ -38,11 +38,6 @@ __if_nametoindex (const char *ifname) > return 0; > #else > struct ifreq ifr; > - int fd = __opensock (); > - > - if (fd < 0) > - return 0; > - > if (strlen (ifname) >= IFNAMSIZ) > { > __set_errno (ENODEV); > @@ -50,6 +45,12 @@ __if_nametoindex (const char *ifname) > } > > strncpy (ifr.ifr_name, ifname, sizeof (ifr.ifr_name)); > + > + int fd = __opensock (); > + > + if (fd < 0) > + return 0; > + > if (__ioctl (fd, SIOCGIFINDEX, &ifr) < 0) > { > int saved_errno = errno; Thanks for the fix, is this going to be in master? Can we take this patch as CVE fix?
* Victor Rodriguez: > Thanks for the fix, is this going to be in master? > Can we take this patch as CVE fix? I've pushed this to master now. There's going to be test cases later. Thanks, Florian
diff --git a/NEWS b/NEWS index f488821af1..1098be1afb 100644 --- a/NEWS +++ b/NEWS @@ -57,7 +57,9 @@ Changes to build and runtime requirements: Security related changes: - [Add security related changes here] + CVE-2018-19591: A file descriptor leak in if_nametoindex can lead to a + denial of service due to resource exhaustion when processing getaddrinfo + calls with crafted host names. Reported by Guido Vranken. The following bugs are resolved with this release: diff --git a/sysdeps/unix/sysv/linux/if_index.c b/sysdeps/unix/sysv/linux/if_index.c index e3d08982d9..3e999d6509 100644 --- a/sysdeps/unix/sysv/linux/if_index.c +++ b/sysdeps/unix/sysv/linux/if_index.c @@ -45,6 +45,7 @@ __if_nametoindex (const char *ifname) if (strlen (ifname) >= IFNAMSIZ) { + __close_nocancel_nostatus (fd); __set_errno (ENODEV); return 0; }