Closed
Bug 70203
Opened 24 years ago
Closed 24 years ago
psm crashes on linux/alpha
Categories
(Core :: Security: PSM, defect)
Tracking
()
VERIFIED
INVALID
People
(Reporter: blizzard, Assigned: javi)
Details
psm is crashing on linux/alpha. This appears to be because of bad assumptions
made about structure sizes and the packing of structures. Specifically the
assumptions about the size of SSMString are pretty scary and the size and
packing of the structure. There are a lot of places where the sizes of the
members of the structure are added together to allocate the entire structure and
then accessed via pointer arithmetic. This is kind of scary.
Here are some examples of what I'm talking about:
http://lxr.mozilla.org/mozilla/source/security/psm/server/sslskst.c#298
http://lxr.mozilla.org/mozilla/source/security/psm/lib/protocol/ssmdefs.h#64
http://lxr.mozilla.org/mozilla/source/security/psm/server/servutil.c#1248
This is done all over the code and I'm not confident enough to go through and
fix all the call sites. Plus, frankly, I don't think that I should have to. :)
Also shouldn't this struct definition use a char * or a char[1]?
/* A length-encoded string. */
struct _SSMString {
CMUint32 m_length;
char m_data;
}
and CMUint32 is an unsigned long which isn't always 32 bits. More misleading
code. Ugh.
Comment 2•24 years ago
|
||
It should be at least char m_data[1];, and the size computation should use
sizeof(SSMString) + length, if the string is NUL-terminated (i.e., if there must
be an extra char for the terminating NUL).
>and CMUint32 is an unsigned long which isn't always 32 bits. More misleading
>code. Ugh.
Why in the world aren't the PRUint32, etc. types used, or at least renamed,
here? Please do not reinvent that wheel (the JS engine did something similar,
but better: it ripped off prtypes.h and all the prcpucfg.h machinery, in order
to stand alone; but it still sucks and I regret it).
/be
Comment 3•24 years ago
|
||
Now that PSM 2 has landed, the code referenced in the report is no longer in the
build.
Status: NEW → RESOLVED
Closed: 24 years ago
Resolution: --- → INVALID
You need to log in
before you can comment on or make changes to this bug.
Description
•