Closed Bug 70203 Opened 24 years ago Closed 24 years ago

psm crashes on linux/alpha

Categories

(Core :: Security: PSM, defect)

Other Branch
Other
Linux
defect
Not set
normal

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.
Reassigning.
Assignee: ddrinan → javi
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
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
Verified invalid.
Status: RESOLVED → VERIFIED
Product: PSM → Core
You need to log in before you can comment on or make changes to this bug.