On (29/10/13 10:05), Jakub Hrozek wrote:
The code wrote into the middle of the packet to a space that was
already
reserved and allocated but then still advanced the pointer to the
buffer.
https://fedorahosted.org/sssd/ticket/2124
From d533bb267ffb1447503f6af671a6d2d2db9ddd10 Mon Sep 17 00:00:00
2001
From: Jakub Hrozek <jhrozek(a)redhat.com>
Date: Tue, 29 Oct 2013 07:10:30 +0100
Subject: [PATCH] NSS: Fix service enumeration
The code wrote into the middle of the packet to a space that was already
reserved and allocated but then still advanced the pointer to the buffer.
https://fedorahosted.org/sssd/ticket/2124
---
src/responder/nss/nsssrv_services.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/responder/nss/nsssrv_services.c b/src/responder/nss/nsssrv_services.c
index 79caa7d08cdfff25112eedab98a0419f1b1d154e..ff588592c814dc43e30f9e57ddab230efff15df2
100644
--- a/src/responder/nss/nsssrv_services.c
+++ b/src/responder/nss/nsssrv_services.c
@@ -752,7 +752,7 @@ fill_service(struct sss_packet *packet,
written_aliases++;
talloc_zfree(tmpstr);
}
- SAFEALIGN_SET_UINT32(&body[aptr], written_aliases, &rsize);
+ SAFEALIGN_SET_UINT32(&body[aptr], written_aliases, NULL);
num++;
}
--
1.8.3.1
Nice catch.
We have already reserved place for variable written_aliases.
Here is code:
-------------
ret = sss_packet_grow(packet, 2 * sizeof(uint16_t)
+ sizeof(uint32_t) // reserve place for variable written_aliases
+ cased_name.len
+ cased_proto.len);
sss_packet_get_body(packet, &body, &blen);
/* Store the port number */
SAFEALIGN_SET_UINT32(&body[rzero + rsize], (uint32_t)htons(port), &rsize);
... snip ...
/* We'll store the alias count here */
aptr = rzero+rsize; <<< remember offset for variable written_aliases
rsize += sizeof(uint32_t); <<< increment rsize
So it does not make sense to increment rsize twice and it was
a reason why we could access behind allocated array.
Please add comment before line, where variable is written to packet,
because it needn't be clear why rsize is not incremented.
I know that this patch will be backported to older branches.
In this case, do we want to use new macro SAFEALIGN_SETMEM_UINT32?
LS