G'day Frank ...
On 09/03/15 07:53, Frank Ch. Eigler wrote:
...
... I think this is a different issue. The one I happened on is this part
of p_pmns.c:
...
This leaves the 0-3 last bytes of the __pmFindPDUBuf result buf
uninintialized, since alloc_len is 0-3 bytes larger than "namelen",
due to alloc_len being larger than necessary to carry the string.
Yep, we may leak 0-3 bytes of "whatever was in the PDU buffer from last
time" across the PDU interface.
This is indeed different to what I was commenting on.
Back to your original questions ...
> Some questions. Why do we pad at all? Should we fix qa/367 to stop
> asserting padding content? Or teach SendNameReq to initialize all
> that padding (see below for that)? Or have __pmFindPDU() memset-0 the
> thing? Letting the uninitialized padding travel would be a small
> security problem, if a pmcd were tricked into leaking information from
> other clients.
PDUs are multiples of an int .. so that's why we're padding here.
I can't see the place in pducheck.c that is making assumptions about
padding ... can you explain that to me?
memset to zero seem like wrong plan of attack ... this is useless work
for the vast majority of cases where all the bytes in the pdu buffer are
explicitly assigned values before being sent. If we need/want to fix it
libpcp, then your suggested patch is fine by me ... '~' is perhaps a
little clearer than 0x7e and matches what we do elsewhere in similar cases.
|