Skip Menu |
 

History Show all quoted text — Show full headers
# Mon Jul 05 10:32:08 2004 Pierre Goyette <pierre@montreal.hcl.com> - Ticket created
Subject: Problem with LSH_DLGINFO_EX_V1_SZ
Date: Mon, 5 Jul 2004 10:32:08 -0400
From: "Pierre Goyette" <pierre@montreal.hcl.com>
To: <kfw-bugs@mit.edu>
Download (untitled) / with headers
text/plain 899B
Download (untitled) / with headers
text/html 4.1KiB
There is a bug in the definition of LSH_DLGINFO_EX_V1_SZ in 2.6.3 and possibly earlier versions.
 
The define is:
 
    #define LSH_DLGINFO_EX_V1_SZ (sizeof(DWORD) + 3 * sizeof(LPSTR) * 8 * sizeof(int))
 
In fact, it should be:
 
    #define LSH_DLGINFO_EX_V1_SZ (sizeof(DWORD) + 3 * sizeof(LPSTR) + 8 * sizeof(int))
 
The size of the V1 structure should be 48 bytes but due to the typo, it thinks the size is 388 bytes.
 
The source in lsh_pwd.c should probably read:
 
  if ((lpdi->size != LSH_DLGINFO_EX_V1_SZ && 
       lpdi->size != sizeof(LSH_DLGINFO_EX)
       lpdi-_size != 388) ||
       lpdi->dlgtype != DLGTYPE_PASSWD) {
    ...
 
to allow existing applications not to generate error messages when the source is updated or when working with versions of the client which use the broken definition.
 
Regards,
 
Pierre Goyette
Senior Director, R&D
Hummingbird Ltd.
Montreal, Quebec, Canada
# Mon Jul 05 22:52:20 2004 jaltman (Jeffrey Altman) - Status changed from 'new' to 'open'
# Mon Jul 05 22:52:20 2004 jaltman (Jeffrey Altman) - Given to jaltman (Jeffrey Altman)
# Mon Jul 05 22:52:21 2004 jaltman (Jeffrey Altman) - Correspondence added
Download (untitled) / with headers
text/plain 72B
Good catch. The appropriate fix will be to define a V2 data structure.
# Mon Jul 05 22:59:45 2004 jaltman (Jeffrey Altman) - Correspondence added
Download (untitled) / with headers
text/plain 310B
Actually, why would an application ever be using this value?

This #define is the value of the old data structure size, not the new
size. Programmers should never be setting the size to that value.
Programmers should always be setting the size to sizeof(struct ...)

I am just going to correct the definition.
# Tue Jul 06 08:49:12 2004 Pierre Goyette <pierre@montreal.hcl.com> - Comments added
Subject: RE: [krbdev.mit.edu #2622] Problem with LSH_DLGINFO_EX_V1_SZ
Date: Tue, 6 Jul 2004 08:48:42 -0400
From: "Pierre Goyette" <pierre@montreal.hcl.com>
To: <rt-kfw-comment@krbdev.mit.edu>
Cc: <jaltman@columbia.edu>
RT-Send-Cc:
Download (untitled) / with headers
text/plain 1.4KiB
Jeff,

Most windows application do something like:

LSH_DLGINFO_EX dlgInfoEx;

dlgInfoEx.size = sizeof( dlgInfoEx );

I realize that this is the size of the old structure (if you are using
the old SDK). The problem is, if you want an application to work with
2.5 & 2.6.x & future versions, it is not possible to pass a correct
value and not get an error. If you pass the real old structure size,
this will work with 2.5 and fail with 2.6.x. If you pass the new 2.6
structure size, it will fail with 2.5. If you pass the definition size,
it will fail with 2.5 and work with 2.6.

The only way for an application to programmatically work properly is to
get the version of LEASHW32.DLL and check if it is 2.6 or higher and
then pass the new dialog structure size. This is pretty ugly to have to
do this. Unless you have another way to determine the version and know
what dialog structure size to pass?

Thanks,

Pierre

Show quoted text
-----Original Message-----
From: Unprivileged W User,,,, [mailto:www@MIT.EDU] On Behalf Of Jeffrey
Altman via RT
Sent: Monday, July 05, 2004 11:00 PM
To: Pierre Goyette
Subject: [krbdev.mit.edu #2622] Problem with LSH_DLGINFO_EX_V1_SZ

Actually, why would an application ever be using this value?

This #define is the value of the old data structure size, not the new
size. Programmers should never be setting the size to that value.
Programmers should always be setting the size to sizeof(struct ...)

I am just going to correct the definition.
# Tue Jul 06 08:52:55 2004 jaltman (Jeffrey Altman) - Comments added
Date: Tue, 06 Jul 2004 08:52:48 -0400
From: Jeffrey Altman <jaltman@mit.edu>
Cc: rt-kfw-comment@krbdev.mit.edu
Subject: Re: [krbdev.mit.edu #2622] Problem with LSH_DLGINFO_EX_V1_SZ
RT-Send-Cc:
Download (untitled) / with headers
text/plain 1.3KiB
The purpose of the size is for the DLL to know how many of the fields are
valid. Clearly there is a bug in 2.6.0 -> 2.6.4 Beta 2. The bug is not
in the
header but in the Leash32.DLL. If the old size is detected, the new fields
are not used. If the current size is detected, they are used.

Why do you believe this is a problem when user's specify

LSH_DLGINFO_EX dlgInfoEx;

dlgInfoEx.size = sizeof( dlgInfoEx );

as they should be doing?


Pierre Goyette wrote:

Show quoted text
>Jeff,
>
>Most windows application do something like:
>
>LSH_DLGINFO_EX dlgInfoEx;
>
>dlgInfoEx.size = sizeof( dlgInfoEx );
>
>I realize that this is the size of the old structure (if you are using
>the old SDK). The problem is, if you want an application to work with
>2.5 & 2.6.x & future versions, it is not possible to pass a correct
>value and not get an error. If you pass the real old structure size,
>this will work with 2.5 and fail with 2.6.x. If you pass the new 2.6
>structure size, it will fail with 2.5. If you pass the definition size,
>it will fail with 2.5 and work with 2.6.
>
>The only way for an application to programmatically work properly is to
>get the version of LEASHW32.DLL and check if it is 2.6 or higher and
>then pass the new dialog structure size. This is pretty ugly to have to
>do this. Unless you have another way to determine the version and know
>what dialog structure size to pass?
>
>Thanks,
>
>Pierre
>
>
# Wed Jul 07 19:01:15 2004 Pierre Goyette <pierre@montreal.hcl.com> - Comments added
Subject: RE: [krbdev.mit.edu #2622] Problem with LSH_DLGINFO_EX_V1_SZ
Date: Wed, 7 Jul 2004 19:00:03 -0400
From: "Pierre Goyette" <pierre@montreal.hcl.com>
To: <rt-kfw-comment@krbdev.mit.edu>
Cc: <jaltman@columbia.edu>
RT-Send-Cc:
Download (untitled) / with headers
text/plain 1.6KiB
Jeff,

I thought about this a bit and I believe the fundamental problem is
actually with the WM_INITDLG code in lsh_pwd.c.

The problem is that the code checks only for two known structure sizes:
the original 2.5 size (V1) and the larger 2.6 size (V2).

The code should in fact check that the size is at least the V1 structure
size or at least the V2 structure size.

Consider the problem where the structure gets resized in a future patch.
Someone cannot write client code which will work with the new clients
and old. If a size larger than required is passed to the API, it should
happily take the structure and ignore members it does not know about.

The first check should read:

if (lpdi->size < LSH_DLGINFO_EX_V1_SZ || lpdi->dlgtype !=
DLGTYPE_PASSWD) {
MessageBox(hDialog, "An incorrect initialization data structure was
provided.", "AuthenticateProc()", MB_OK | MB_ICONSTOP);
return FALSE;
}

// Then, the check should be

if ( lpdi->size >= sizeof(LSH_DLGINFO_EX) ) {
// Access the V2 members safely
}

If the structure size is large enough to contain the new members, then
do what you need to.

Comments?

Pierre

Show quoted text
> -----Original Message-----
> From: Unprivileged W User,,,, [mailto:www@MIT.EDU] On Behalf
> Of Jeffrey Altman via RT
> Sent: Monday, July 05, 2004 11:00 PM
> To: Pierre Goyette
> Subject: [krbdev.mit.edu #2622] Problem with LSH_DLGINFO_EX_V1_SZ
>
> Actually, why would an application ever be using this value?
>
> This #define is the value of the old data structure size, not
> the new size. Programmers should never be setting the size
> to that value.
> Programmers should always be setting the size to sizeof(struct ...)
>
> I am just going to correct the definition.
>
# Wed Jul 07 20:23:49 2004 "Jeffrey Altman [Kermit Project]" <jaltman@columbia.edu> - Comments added
Date: Wed, 07 Jul 2004 20:23:47 -0400
From: Jeffrey Altman <jaltman@columbia.edu>
To: Pierre Goyette <pierre@montreal.hcl.com>
Cc: rt-kfw-comment@krbdev.mit.edu
Subject: Re: [krbdev.mit.edu #2622] Problem with LSH_DLGINFO_EX_V1_SZ
RT-Send-Cc:
Download (untitled) / with headers
text/plain 439B
You have now described the intended behavior.
The test is now:

if ((lpdi->size != LSH_DLGINFO_EX_V1_SZ &&
lpdi->size < sizeof(LSH_DLGINFO_EX)) ||
lpdi->dlgtype != DLGTYPE_PASSWD) {

MessageBox(hDialog, "An incorrect initialization data
structure was provided.",
"AuthenticateProc()",
MB_OK | MB_ICONSTOP);
return FALSE;
}
Download smime.p7s
application/x-pkcs7-signature 3.1KiB

Message body not shown because it is not plain text.

# Wed Jul 07 20:31:51 2004 Pierre Goyette <pierre@montreal.hcl.com> - Comments added
Subject: RE: [krbdev.mit.edu #2622] Problem with LSH_DLGINFO_EX_V1_SZ
Date: Wed, 7 Jul 2004 20:30:54 -0400
From: "Pierre Goyette" <pierre@montreal.hcl.com>
To: "Jeffrey Altman" <jaltman@columbia.edu>
Cc: <rt-kfw-comment@krbdev.mit.edu>
RT-Send-Cc:
Download (untitled) / with headers
text/plain 1.3KiB
You still have the problem that if the structure size were to increase
in the future and someone built code using the new larger structure, it
would not work with older client code. All instances of:

if ( lpdi->size == sizeof(LSH_DLGINFO_EX) ) {

Should read:

if ( lpdi->size >= sizeof(LSH_DLGINFO_EX) ) {

The checks done should verify that the structure is *at least* a given
size. It does not need to check that it is an exact size.

This is how many Windows APIs are coded. This allows newer programs to
be backward compatible with older O/S'. The older O/S checks that the
structure is large enough to handle its requirements. It doesn't care if
a structure is larger.

Pierre

Show quoted text
> -----Original Message-----
> From: Jeffrey Altman [mailto:jaltman@columbia.edu]
> Sent: Wednesday, July 07, 2004 8:24 PM
> To: Pierre Goyette
> Cc: rt-kfw-comment@krbdev.mit.edu
> Subject: Re: [krbdev.mit.edu #2622] Problem with LSH_DLGINFO_EX_V1_SZ
>
> You have now described the intended behavior.
> The test is now:
>
> if ((lpdi->size != LSH_DLGINFO_EX_V1_SZ &&
> lpdi->size < sizeof(LSH_DLGINFO_EX)) ||
> lpdi->dlgtype != DLGTYPE_PASSWD) {
>
> MessageBox(hDialog, "An incorrect initialization
> data structure was provided.",
> "AuthenticateProc()",
> MB_OK | MB_ICONSTOP);
> return FALSE;
> }
>
>
>
# Wed Jul 07 20:35:35 2004 "Jeffrey Altman [Kermit Project]" <jaltman@columbia.edu> - Comments added
Date: Wed, 07 Jul 2004 20:35:28 -0400
From: Jeffrey Altman <jaltman@columbia.edu>
To: Pierre Goyette <pierre@montreal.hcl.com>
Cc: rt-kfw-comment@krbdev.mit.edu
Subject: Re: [krbdev.mit.edu #2622] Problem with LSH_DLGINFO_EX_V1_SZ
RT-Send-Cc:
Download (untitled) / with headers
text/plain 2.4KiB
cvs diff: Diffing .
Index: lsh_pwd.c
===================================================================
RCS file: /cvs/pismere/pismere/athena/auth/leash/leashdll/lsh_pwd.c,v
retrieving revision 1.46
diff -u -w -r1.46 lsh_pwd.c
--- lsh_pwd.c 3 May 2004 21:29:48 -0000 1.46
+++ lsh_pwd.c 8 Jul 2004 00:34:52 -0000
@@ -1305,7 +1305,7 @@
*( (LPLSH_DLGINFO_EX far *)(&lpdi) ) =
(LPLSH_DLGINFO_EX)(LPSTR)lParam;


if ((lpdi->size != LSH_DLGINFO_EX_V1_SZ &&
- lpdi->size != sizeof(LSH_DLGINFO_EX)) ||
+ lpdi->size < sizeof(LSH_DLGINFO_EX)) ||
lpdi->dlgtype != DLGTYPE_PASSWD) {

MessageBox(hDialog, "An incorrect initialization
data st
ructure was provided.",
@@ -1314,7 +1314,7 @@
return FALSE;
}

- if ( lpdi->size == sizeof(LSH_DLGINFO_EX) ) {
+ if ( lpdi->size >= sizeof(LSH_DLGINFO_EX) ) {
lpdi->out.username[0] = 0;
lpdi->out.realm[0] = 0;
}
@@ -1752,7 +1752,7 @@
Leash_set_default_noaddresses(noaddresses);
}

- if ( lpdi->size == sizeof(LSH_DLGINFO_EX) ) {
+ if ( lpdi->size >= sizeof(LSH_DLGINFO_EX) ) {
strncpy(lpdi->out.username, username,
LEASH_USERNAME_SZ);
lpdi->out.username[LEASH_USERNAME_SZ-1] = 0;
strncpy(lpdi->out.realm, realm, LEASH_REALM_SZ);
@@ -1814,7 +1814,7 @@

*( (LPLSH_DLGINFO_EX far *)(&lpdi) ) =
(LPLSH_DLGINFO_EX)(LPSTR)lParam;


- if ((lpdi->size != sizeof(LSH_DLGINFO_EX) &&
+ if ((lpdi->size < sizeof(LSH_DLGINFO_EX) &&
lpdi->size != LSH_DLGINFO_EX_V1_SZ) ||
lpdi->dlgtype != DLGTYPE_CHPASSWD) {

@@ -1824,7 +1824,7 @@
return FALSE;
}

- if ( lpdi->size == sizeof(LSH_DLGINFO_EX) ) {
+ if ( lpdi->size >= sizeof(LSH_DLGINFO_EX) ) {
lpdi->out.username[0] = 0;
lpdi->out.realm[0] = 0;
}
@@ -2105,7 +2105,7 @@
return TRUE;
}

- if ( lpdi->size == sizeof(LSH_DLGINFO_EX) ) {
+ if ( lpdi->size >= sizeof(LSH_DLGINFO_EX) ) {
strncpy(lpdi->out.username, username,
LEASH_USERNAME_SZ);
lpdi->out.username[LEASH_USERNAME_SZ-1] = 0;
strncpy(lpdi->out.realm, realm, LEASH_REALM_SZ);
Download smime.p7s
application/x-pkcs7-signature 3.1KiB

Message body not shown because it is not plain text.

# Mon Jul 12 11:00:15 2004 jaltman (Jeffrey Altman) - Status changed from 'open' to 'resolved'
# Mon Jul 12 11:00:15 2004 jaltman (Jeffrey Altman) - Comments added
Download (untitled) / with headers
text/plain 31B
patches applied to 2.6.4 beta 4