Bug 238022 - buffer overrun in function make_request in sbin/dhclient/dhclient.c
Summary: buffer overrun in function make_request in sbin/dhclient/dhclient.c
Status: New
Alias: None
Product: Base System
Classification: Unclassified
Component: kern (show other bugs)
Version: CURRENT
Hardware: Any Any
: --- Affects Many People
Assignee: freebsd-bugs mailing list
URL:
Keywords: patch
Depends on:
Blocks:
 
Reported: 2019-05-21 13:14 UTC by Young
Modified: 2019-12-07 03:56 UTC (History)
3 users (show)

See Also:


Attachments
Proposed patch (1.13 KB, patch)
2019-05-21 13:14 UTC, Young
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Young 2019-05-21 13:14:52 UTC
Created attachment 204510 [details]
Proposed patch

There is a buffer overrun vulnerability in function make_request of sbin/dhclient/dhclient.c, which is similar to the vulnerability that was fixed in https://github.com/freebsd/freebsd/commit/16b93d101357f716946014207ddfe9d849f97fc9.

        /* set unique client identifier */
        char client_ident[sizeof(struct hardware)];
        if (!options[DHO_DHCP_CLIENT_IDENTIFIER]) {
                int hwlen = (ip->hw_address.hlen < sizeof(client_ident)-1) ?
                                ip->hw_address.hlen : sizeof(client_ident)-1;
                client_ident[0] = ip->hw_address.htype;
                memcpy(&client_ident[1], ip->hw_address.haddr, hwlen);
                options[DHO_DHCP_CLIENT_IDENTIFIER] = &option_elements[DHO_DHCP_CLIENT_IDENTIFIER];
                options[DHO_DHCP_CLIENT_IDENTIFIER]->value = client_ident;
                options[DHO_DHCP_CLIENT_IDENTIFIER]->len = hwlen+1;
                options[DHO_DHCP_CLIENT_IDENTIFIER]->buf_size = hwlen+1;
                options[DHO_DHCP_CLIENT_IDENTIFIER]->timeout = 0xFFFFFFFF;
        }

A DHCP client identifier is simply the hardware type (one byte) concatenated with the hardware address.
We should set the lengthe of clinet_ident to sizeof(ip->hw_address.haddr) + 1, instead of sizeof(struct hardware).

The attachment is the proposed patch.
Comment 1 Dave Cottlehuber freebsd_committer 2019-11-28 23:50:14 UTC
LGTM. cem@ seeing as you committed the last patch like this can you do it here, and MFC too?
Comment 2 Conrad Meyer freebsd_committer 2019-11-29 03:32:21 UTC
I put it in CURRENT but someone else can MFC if they like.
Comment 3 commit-hook freebsd_committer 2019-11-29 03:32:31 UTC
A commit references this bug:

Author: cem
Date: Fri Nov 29 03:31:47 UTC 2019
New revision: 355204
URL: https://svnweb.freebsd.org/changeset/base/355204

Log:
  Fix braino in previous bugfix r300174

  The previous revision missed the exact same error in a copy paste block
  of the same code in another function.  Fix the identical case, too.

  A DHCP client identifier is simply the hardware type (one byte)
  concatenated with the hardware address (some variable number of bytes,
  but at most 16).  Limit the size of the temporary buffer to match and
  the rest of the calculations shake out correctly.

  PR:		238022
  Reported by:	Young <yangx92 AT hotmail.com>
  Submitted by:	Young <yangx92 AT hotmail.com>
  MFC after:	I don't plan to but you should feel free
  Security:	yes

Changes:
  head/sbin/dhclient/dhclient.c
Comment 4 commit-hook freebsd_committer 2019-12-07 03:56:45 UTC
A commit references this bug:

Author: emaste
Date: Sat Dec  7 03:56:37 UTC 2019
New revision: 355482
URL: https://svnweb.freebsd.org/changeset/base/355482

Log:
  MFC r238022 (cem): dhclient: fix braino in previous bugfix r300174

  The previous revision missed the exact same error in a copy paste block
  of the same code in another function.  Fix the identical case, too.

  A DHCP client identifier is simply the hardware type (one byte)
  concatenated with the hardware address (some variable number of bytes,
  but at most 16).  Limit the size of the temporary buffer to match and
  the rest of the calculations shake out correctly.

  PR:		238022
  Reported by:	Young <yangx92 AT hotmail.com>
  Submitted by:	Young <yangx92 AT hotmail.com>

Changes:
_U  stable/12/
  stable/12/sbin/dhclient/dhclient.c