Re: [Linuxptp-devel] [PATCH] port: fix buffer overflow in net_sync_resp_append()

[Richard Cochran]

On Fri, Apr 06, 2018 at 05:13:11PM +0200, Miroslav Lichvar wrote:
> That's definitely better. Will you fix the patch, or would you like me
> to send v2?

I'll fix it.

Thanks,
Richard

--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
Linuxptp-devel mailing list
Linuxptp-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel

Re: [Linuxptp-devel] [PATCH] port: fix buffer overflow in net_sync_resp_append()

[Richard Cochran]

On Fri, Apr 06, 2018 at 12:30:08PM +0200, Miroslav Lichvar wrote:
> The PortAddress structure has no space for the actual address and should
> be used only as a pointer to a larger buffer.

Oh man, Sloppy!  Time for 1.9.2.  
 
> @@ -403,32 +403,34 @@ static int net_sync_resp_append(struct port *p, struct 
> ptp_message *m)
>   struct port *best = clock_best_port(p->clock);
>   struct nsm_resp_tlv_head *head;
>   struct Timestamp last_sync;
> - struct PortAddress paddr;
> + struct PortAddress *paddr;
>   struct ptp_message *tmp;
>   struct tlv_extra *extra;
>   unsigned char *ptr;
> + char buf[sizeof(*paddr) + 16];

Sure, 16 is large enough for a 128 bit ipv6 address, but I'd like this
to be explicit.

char buf[sizeof(*paddr) + sizeof(struct sockaddr_storage)];

How about this?

Thanks,
Richard

--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
Linuxptp-devel mailing list
Linuxptp-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel

[Linuxptp-devel] [PATCH] port: fix buffer overflow in net_sync_resp_append()

[Miroslav Lichvar]

The PortAddress structure has no space for the actual address and should
be used only as a pointer to a larger buffer.

The issue was reported by gcc with enabled source fortification.

Signed-off-by: Miroslav Lichvar 
---
 port.c | 28 +++-
 1 file changed, 15 insertions(+), 13 deletions(-)

diff --git a/port.c b/port.c
index cee6445..872c7be 100644
--- a/port.c
+++ b/port.c
@@ -403,32 +403,34 @@ static int net_sync_resp_append(struct port *p, struct 
ptp_message *m)
struct port *best = clock_best_port(p->clock);
struct nsm_resp_tlv_head *head;
struct Timestamp last_sync;
-   struct PortAddress paddr;
+   struct PortAddress *paddr;
struct ptp_message *tmp;
struct tlv_extra *extra;
unsigned char *ptr;
+   char buf[sizeof(*paddr) + 16];
int tlv_len;
 
last_sync = tmv_to_Timestamp(clock_ingress_time(p->clock));
pid = dad->pds.parentPortIdentity.clockIdentity;
+   paddr = (struct PortAddress *)buf;
 
if (best && memcmp(, , sizeof(cid))) {
/* Extract the parent's protocol address. */
-   paddr.networkProtocol = transport_type(best->trp);
-   paddr.addressLength =
-   transport_protocol_addr(best->trp, paddr.address);
+   paddr->networkProtocol = transport_type(best->trp);
+   paddr->addressLength =
+   transport_protocol_addr(best->trp, paddr->address);
if (best->best) {
tmp = TAILQ_FIRST(>best->messages);
-   extract_address(tmp, );
+   extract_address(tmp, paddr);
}
} else {
/* We are our own parent. */
-   paddr.networkProtocol = transport_type(p->trp);
-   paddr.addressLength =
-   transport_protocol_addr(p->trp, paddr.address);
+   paddr->networkProtocol = transport_type(p->trp);
+   paddr->addressLength =
+   transport_protocol_addr(p->trp, paddr->address);
}
 
-   tlv_len = sizeof(*head) + sizeof(*extra->foot) + paddr.addressLength;
+   tlv_len = sizeof(*head) + sizeof(*extra->foot) + paddr->addressLength;
 
extra = msg_tlv_append(m, tlv_len);
if (!extra) {
@@ -439,12 +441,12 @@ static int net_sync_resp_append(struct port *p, struct 
ptp_message *m)
head->type = TLV_PTPMON_RESP;
head->length = tlv_len - sizeof(head->type) - sizeof(head->length);
head->port_state = p->state == PS_GRAND_MASTER ? PS_MASTER : p->state;
-   head->parent_addr.networkProtocol = paddr.networkProtocol;
-   head->parent_addr.addressLength = paddr.addressLength;
-   memcpy(head->parent_addr.address, paddr.address, paddr.addressLength);
+   head->parent_addr.networkProtocol = paddr->networkProtocol;
+   head->parent_addr.addressLength = paddr->addressLength;
+   memcpy(head->parent_addr.address, paddr->address, paddr->addressLength);
 
ptr = (unsigned char *) head;
-   ptr += sizeof(*head) + paddr.addressLength;
+   ptr += sizeof(*head) + paddr->addressLength;
extra->foot = (struct nsm_resp_tlv_foot *) ptr;
 
memcpy(>foot->parent, >pds, sizeof(extra->foot->parent));
-- 
2.14.3


--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
Linuxptp-devel mailing list
Linuxptp-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel