On 11/14/2013 01:14 PM, Lukas Slebodnik wrote:>>From
fbc7adbf1331fdb931b43f2b6ffda43565ce442e Mon Sep 17 00:00:00 2001
> >From: Michal Zidek<mzidek(a)redhat.com>
> >Date: Wed, 4 Sep 2013 16:17:57 +0200
> >Subject: [PATCH 5/7] responder: Use SAFEALIGN macro when checking
pam
data
> >validity.
> >
> >resolves:
> >https://fedorahosted.org/sssd/ticket/1359
> >---
> >src/responder/pam/pamsrv_cmd.c | 18 +++++++++++++-----
> >1 file changed, 13 insertions(+), 5 deletions(-)
> >
> >diff --git a/src/responder/pam/pamsrv_cmd.c
b/src/responder/pam/pamsrv_cmd.c
> >index 32efc4b..6418047 100644
> >--- a/src/responder/pam/pamsrv_cmd.c
> >+++ b/src/responder/pam/pamsrv_cmd.c
> >@@ -144,12 +144,20 @@ static int pam_parse_in_data_v2(struct
sss_domain_info *domains,
> > uint32_t size;
> > char *pam_user;
> > int ret;
> >- uint32_t terminator = SSS_END_OF_PAM_REQUEST;
> >+ uint32_t start;
> >+ uint32_t terminator;
> >
> >- if (blen < 4*sizeof(uint32_t)+2 ||
> >- ((uint32_t *)body)[0] != SSS_START_OF_PAM_REQUEST ||
> >- memcmp(&body[blen - sizeof(uint32_t)], &terminator,
sizeof(uint32_t)) != 0) {
Function memcmp is also used in the function
pam_forwarder_parse_data,
although there is no alignment warning we should be consistent at
least in the
same file.
Yes. I modified it to use safealign copy as well in the new patch.
Michal
PS:
Btw. why is the condition to detect unterminated data like this
if (blen >= sizeof(uint32_t)
&& lastfourbytes != END_OF_PAM_REQUEST) {
return error;
}
Shouldn't it be like this?
if (blen < sizeof(uint32_t)
|| lastfourbytes != END_OF_PAM_REQUEST) {
return error;
}
Is it possible that pam request shorter than terminator is valid?