[U-Boot] Remote code execution vulnerabilities in U-Boot's NFS and other IP parsing code
Simon Goldschmidt
simon.k.r.goldschmidt at gmail.com
Wed Jul 24 10:52:23 UTC 2019
On Tue, Jul 23, 2019 at 4:02 PM Fermín Serna <fermin at semmle.com> wrote:
>
> Of course, sorry about that.
> Please note this was a quick untested patch used for highlighting the
> vulnerabilities. I would start with it but most likely needs some
> extra work. At this time, I would appreciate someone else to take it
> form here.
>
> See inline below.
Thanks.
In which form do you expect us to take over, regarding patch
authorship? From a first look, at least coding style changes are
needed, but some other changes would probably be done before
applying as well.
Regards,
Simon
>
> diff --git a/net/net.c b/net/net.c
> index 58b0417cbe..9957450392 100644
> --- a/net/net.c
> +++ b/net/net.c
> @@ -1256,6 +1256,12 @@ void net_process_received_packet(uchar
> *in_packet, int len)
> "received UDP (to=%pI4, from=%pI4, len=%d)\n",
> &dst_ip, &src_ip, len);
>
> + if ((ntohs(ip->udp_len) < UDP_HDR_SIZE) ||
> + (ntohs(ip->udp_len) > len)) {
> + printf(" Invalid IP->udp_len field\n");
> + return;
> + }
> +
> #ifdef CONFIG_UDP_CHECKSUM
> if (ip->udp_xsum != 0) {
> ulong xsum;
> @@ -1305,6 +1311,7 @@ void net_process_received_packet(uchar
> *in_packet, int len)
> ntohs(ip->udp_src),
> ntohs(ip->udp_len) - UDP_HDR_SIZE);
> #endif
> +
> /*
> * IP header OK. Pass the packet to the current handler.
> */
> diff --git a/net/nfs.c b/net/nfs.c
> index d6a7f8e827..da6fd76327 100644
> --- a/net/nfs.c
> +++ b/net/nfs.c
> @@ -48,12 +48,12 @@
> static int fs_mounted;
> static unsigned long rpc_id;
> static int nfs_offset = -1;
> -static int nfs_len;
> +static unsigned int nfs_len;
> static ulong nfs_timeout = NFS_TIMEOUT;
>
> static char dirfh[NFS_FHSIZE]; /* NFSv2 / NFSv3 file handle of directory */
> static char filefh[NFS3_FHSIZE]; /* NFSv2 / NFSv3 file handle */
> -static int filefh3_length; /* (variable) length of filefh when NFSv3 */
> +static unsigned int filefh3_length; /* (variable) length of filefh
> when NFSv3 */
>
> static enum net_loop_state nfs_download_state;
> static struct in_addr nfs_server_ip;
> @@ -432,7 +432,8 @@ static int rpc_lookup_reply(int prog, uchar *pkt,
> unsigned len)
> {
> struct rpc_t rpc_pkt;
>
> - memcpy(&rpc_pkt.u.data[0], pkt, len);
> + memset(&rpc_pkt.u.data[0], 0, sizeof(struct rpc_t));
> + memcpy(&rpc_pkt.u.data[0], pkt, min(len, sizeof(struct rpc_t)));
>
> debug("%s\n", __func__);
>
> @@ -464,7 +465,8 @@ static int nfs_mount_reply(uchar *pkt, unsigned len)
>
> debug("%s\n", __func__);
>
> - memcpy(&rpc_pkt.u.data[0], pkt, len);
> + memset(&rpc_pkt.u.data[0], 0, sizeof(struct rpc_t));
> + memcpy(&rpc_pkt.u.data[0], pkt, min(len, sizeof(struct rpc_t)));
>
> if (ntohl(rpc_pkt.u.reply.id) > rpc_id)
> return -NFS_RPC_ERR;
> @@ -490,7 +492,8 @@ static int nfs_umountall_reply(uchar *pkt, unsigned len)
>
> debug("%s\n", __func__);
>
> - memcpy(&rpc_pkt.u.data[0], pkt, len);
> + memset(&rpc_pkt.u.data[0], 0, sizeof(struct rpc_t));
> + memcpy(&rpc_pkt.u.data[0], pkt, min(len, sizeof(struct rpc_t)));
>
> if (ntohl(rpc_pkt.u.reply.id) > rpc_id)
> return -NFS_RPC_ERR;
> @@ -514,7 +517,8 @@ static int nfs_lookup_reply(uchar *pkt, unsigned len)
>
> debug("%s\n", __func__);
>
> - memcpy(&rpc_pkt.u.data[0], pkt, len);
> + memset(&rpc_pkt.u.data[0], 0, sizeof(struct rpc_t));
> + memcpy(&rpc_pkt.u.data[0], pkt, min(len, sizeof(struct rpc_t)));
>
> if (ntohl(rpc_pkt.u.reply.id) > rpc_id)
> return -NFS_RPC_ERR;
> @@ -608,12 +612,13 @@ static int nfs3_get_attributes_offset(uint32_t *data)
> static int nfs_readlink_reply(uchar *pkt, unsigned len)
> {
> struct rpc_t rpc_pkt;
> - int rlen;
> - int nfsv3_data_offset = 0;
> + unsigned int rlen;
> + unsigned int nfsv3_data_offset = 0;
>
> debug("%s\n", __func__);
>
> - memcpy((unsigned char *)&rpc_pkt, pkt, len);
> + memset(&rpc_pkt.u.data[0], 0, sizeof(struct rpc_t));
> + memcpy(&rpc_pkt.u.data[0], pkt, min(len, sizeof(struct rpc_t)));
>
> if (ntohl(rpc_pkt.u.reply.id) > rpc_id)
> return -NFS_RPC_ERR;
> @@ -639,15 +644,23 @@ static int nfs_readlink_reply(uchar *pkt, unsigned len)
>
> strcat(nfs_path, "/");
> pathlen = strlen(nfs_path);
> +
> + if (rlen > sizeof(nfs_path_buff) - pathlen)
> + return -NFS_RPC_ERR;
> +
> memcpy(nfs_path + pathlen,
> (uchar *)&(rpc_pkt.u.reply.data[2 + nfsv3_data_offset]),
> rlen);
> - nfs_path[pathlen + rlen] = 0;
> + nfs_path[pathlen + rlen - 1] = 0;
> } else {
> +
> + if (rlen > sizeof(nfs_path_buff))
> + return -NFS_RPC_ERR;
> +
> memcpy(nfs_path,
> (uchar *)&(rpc_pkt.u.reply.data[2 + nfsv3_data_offset]),
> rlen);
> - nfs_path[rlen] = 0;
> + nfs_path[rlen - 1] = 0;
> }
> return 0;
> }
> @@ -655,12 +668,13 @@ static int nfs_readlink_reply(uchar *pkt, unsigned len)
> static int nfs_read_reply(uchar *pkt, unsigned len)
> {
> struct rpc_t rpc_pkt;
> - int rlen;
> + unsigned int rlen;
> uchar *data_ptr;
>
> debug("%s\n", __func__);
>
> - memcpy(&rpc_pkt.u.data[0], pkt, sizeof(rpc_pkt.u.reply));
> + memset(&rpc_pkt.u.data[0], 0, sizeof(struct rpc_t));
> + memcpy(&rpc_pkt.u.data[0], pkt, min(len, sizeof(struct rpc_t)));
>
> if (ntohl(rpc_pkt.u.reply.id) > rpc_id)
> return -NFS_RPC_ERR;
> @@ -687,6 +701,11 @@ static int nfs_read_reply(uchar *pkt, unsigned len)
> if (supported_nfs_versions & NFSV2_FLAG) {
> rlen = ntohl(rpc_pkt.u.reply.data[18]);
> data_ptr = (uchar *)&(rpc_pkt.u.reply.data[19]);
> +
> + // Make sure at rlen at least does not goes OOB.
> + if (rlen > sizeof(struct rpc_t) - 19 * sizeof(uint32_t))
> + return -NFS_RPC_ERR;
> +
> } else { /* NFSV3_FLAG */
> int nfsv3_data_offset =
> nfs3_get_attributes_offset(rpc_pkt.u.reply.data);
> @@ -699,6 +718,11 @@ static int nfs_read_reply(uchar *pkt, unsigned len)
> */
> data_ptr = (uchar *)
> &(rpc_pkt.u.reply.data[4 + nfsv3_data_offset]);
> +
> + // Make sure at rlen at least does not goes OOB.
> + if (rlen > sizeof(struct rpc_t) - (4 +
> nfsv3_data_offset]) * sizeof(uint32_t))
> + return -NFS_RPC_ERR;
> +
> }
>
> if (store_block(data_ptr, nfs_offset, rlen))
>
> On Tue, 23 Jul 2019 at 02:10, Simon Goldschmidt
> <simon.k.r.goldschmidt at gmail.com> wrote:
> >
> > On Tue, Jul 23, 2019 at 1:09 AM Fermín Serna <fermin at semmle.com> wrote:
> > >
> > > Hello,
> > >
> > > Find attached more information about 13 vulnerabilities we found at
> > > U-Boot and its NFS and networking code. Also, find attached a proposed
> > > quick patch that should serve as a first initial one and should
> > > probably go through iterations of code review.
> > >
> > > Please note, these vulnerabilities are not patched yet at the source
> > > repository. Tom Rini (U-boot's master custodian) requested the
> > > attached report to be published at this mailing list. At this time,
> > > and because of this email, we consider these vulnerabilities public.
> >
> > Would you mind sending the patch again as plain text mail so it can undergo a
> > proper review process on this list?
> >
> > Regards,
> > Simon
> >
> > >
> > > For reference, MITRE has issued CVEs for the vulnerabilities:
> > > CVE-2019-14192, CVE-2019-14193, CVE-2019-14194, CVE-2019-14195,
> > > CVE-2019-14196, CVE-2019-14197, CVE-2019-14198, CVE-2019-14199,
> > > CVE-2019-14200, CVE-2019-14201, CVE-2019-14202, CVE-2019-14203 and
> > > CVE-2019-14204
> > >
> > > Best regards,
> > > --
> > > Fermin
> > > Semmle Security Research Team
More information about the U-Boot
mailing list