| Message ID | 1469591389-9033-1-git-send-email-nikunj@linux.vnet.ibm.com |
|---|---|
| State | Rejected |
| Headers | show |
On 27.07.2016 05:49, Nikunj A Dadhania wrote: > Add missing check to see that the IP offered is for this mac address. > > Signed-off-by: Nikunj A Dadhania <nikunj@linux.vnet.ibm.com> > --- > clients/net-snk/app/netlib/dhcp.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/clients/net-snk/app/netlib/dhcp.c b/clients/net-snk/app/netlib/dhcp.c > index 7e2e88c..3f45633 100644 > --- a/clients/net-snk/app/netlib/dhcp.c > +++ b/clients/net-snk/app/netlib/dhcp.c Please note that this file has recently been moved to lib/libnet/ instead. > @@ -865,6 +865,8 @@ int8_t handle_dhcp(int fd, uint8_t * packet, int32_t packetsize) > switch (dhcp_state) { > case DHCP_STATE_SELECT : > if (opt.msg_type == DHCPOFFER) { > + if(memcmp(btph->chaddr, get_mac_address(), 6)) > + break; > dhcp_own_ip = htonl(btph -> yiaddr); > dhcp_server_ip = opt.server_ID; > dhcp_send_request(fd); > Checking the MAC here should be fine, I think. I'm just wondering: Did you encounter a real world problem here, or did you just find this by reading the sources? The SLOF code already checks the XID for received packets, so that should already give a basic protection against wrongly received broadcast DHCPOFFER messages, shouldn't it? Anyway, let's be better safe than sorry, so including this additional check is certainly a good idea! Thomas
Thomas Huth <thuth@redhat.com> writes: > On 27.07.2016 05:49, Nikunj A Dadhania wrote: >> Add missing check to see that the IP offered is for this mac address. >> >> Signed-off-by: Nikunj A Dadhania <nikunj@linux.vnet.ibm.com> >> --- >> clients/net-snk/app/netlib/dhcp.c | 2 ++ >> 1 file changed, 2 insertions(+) >> >> diff --git a/clients/net-snk/app/netlib/dhcp.c b/clients/net-snk/app/netlib/dhcp.c >> index 7e2e88c..3f45633 100644 >> --- a/clients/net-snk/app/netlib/dhcp.c >> +++ b/clients/net-snk/app/netlib/dhcp.c > > Please note that this file has recently been moved to lib/libnet/ instead. Sure, will update. >> @@ -865,6 +865,8 @@ int8_t handle_dhcp(int fd, uint8_t * packet, int32_t packetsize) >> switch (dhcp_state) { >> case DHCP_STATE_SELECT : >> if (opt.msg_type == DHCPOFFER) { >> + if(memcmp(btph->chaddr, get_mac_address(), 6)) >> + break; >> dhcp_own_ip = htonl(btph -> yiaddr); >> dhcp_server_ip = opt.server_ID; >> dhcp_send_request(fd); >> > > Checking the MAC here should be fine, I think. I'm just wondering: Did > you encounter a real world problem here, or did you just find this by > reading the sources? An issue was reported to me, though the tester hasnt been able to recreate the issue. I had this patch queued for him to test. > The SLOF code already checks the XID for received packets, so that > should already give a basic protection against wrongly received > broadcast DHCPOFFER messages, shouldn't it? Oh right and I have verified that this patch is missing in that tree. > Anyway, let's be better safe than sorry, so including this additional > check is certainly a good idea! Was there with me for few weeks until I could get a confirmation that this indeed is the fix. Then thought of sending it on the list for review ! Regards Nikunj
diff --git a/clients/net-snk/app/netlib/dhcp.c b/clients/net-snk/app/netlib/dhcp.c index 7e2e88c..3f45633 100644 --- a/clients/net-snk/app/netlib/dhcp.c +++ b/clients/net-snk/app/netlib/dhcp.c @@ -865,6 +865,8 @@ int8_t handle_dhcp(int fd, uint8_t * packet, int32_t packetsize) switch (dhcp_state) { case DHCP_STATE_SELECT : if (opt.msg_type == DHCPOFFER) { + if(memcmp(btph->chaddr, get_mac_address(), 6)) + break; dhcp_own_ip = htonl(btph -> yiaddr); dhcp_server_ip = opt.server_ID; dhcp_send_request(fd);
Add missing check to see that the IP offered is for this mac address. Signed-off-by: Nikunj A Dadhania <nikunj@linux.vnet.ibm.com> --- clients/net-snk/app/netlib/dhcp.c | 2 ++ 1 file changed, 2 insertions(+)