On Tue, Jul 23, 2019 at 4:02 PM Fermín Serna <fer...@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.goldschm...@gmail.com> wrote: > > > > On Tue, Jul 23, 2019 at 1:09 AM Fermín Serna <fer...@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 _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot