PR_InitializeNetAddr() does not clear trailing garbage

VERIFIED FIXED in 4.1.1

Status

defect
P3
critical
VERIFIED FIXED
19 years ago
19 years ago

People

(Reporter: taya, Assigned: wtc)

Tracking

4.1.1
All
NetBSD
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(5 attachments)

Reporter

Description

19 years ago
PR_InitializeNetAddr() does not clear addr->sin_zero.
This cause PR_Bind() failure in SSM_OpenPort() at security/psm/server/servutil.c
I think every part which sets address to PRNetAddr should clear whole buffer
before set address.
I also tested on FreeBSD-3.3 and got same result.
Reporter

Comment 1

19 years ago
Reporter

Updated

19 years ago
Blocks: 54797
Assignee

Comment 2

19 years ago
Thanks for the bug report.  We are aware of
this problem of PR_InitializeNetAddr() and
in hindsight we should have had the function
zero the whole PRNetAddr before setting the
members.  We can't change the function to
clear the PRNetAddr because that will change
the behavior of the function.

So callers of PR_InitializeNetAddr will
have to zero the PRNetAddr themselves.
You need to add something like
    memset(&addr, 0, sizeof(PRNetAddr));
to the PSM code before the PR_InitializeNetAddr
call.
Status: UNCONFIRMED → RESOLVED
Closed: 19 years ago
Resolution: --- → WONTFIX
Assignee

Comment 3

19 years ago
I'm now convinced that this bug should be fixed
in NSPR rather than having all the apps work
around it.

I need to convince myself that no apps may depend
on this bug.
Status: RESOLVED → UNCONFIRMED
Resolution: WONTFIX → ---
rubber-stamp confirm. cluefull reporter. Shin'ichiro TAYA, ask somebody from QA
to increase your rights.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Assignee

Comment 5

19 years ago
Assignee

Comment 6

19 years ago
John, I'd like you to review my patch (id=23603).

1. I only zero the sock addr when the 'val' argument
   to PR_InitializeNetAddr or PR_SetNetAddr is not
   PR_IpAddrNull because in the PR_IpAddrNull case
   we need to keep the IP address field intact.

2. I could zero the whole PRNetAddr, but I only zero
   the PRNetAddr union member in use for two reasons:
   - PRNetAddr contains the Unix domain sock addr on
     Unix, which is 108 bytes.  In contract, the 'inet'
     member is only 16 bytes and the 'ipv6' member is
     only 32 bytes.
   - I am concerned that some existing code may actually
     pass a sockaddr_in structure in, so I am concerned
     about writing beyond the end of the sockaddr_in structure.
     This is probably not worth worrying about.

I am very concerned that this patch breaks binary compatibility
because PR_InitializeNetAddr and PR_SetNetAddr is not zeroing
the sock addr right now.  What do you think?  Do you think we
should modify PR_InitializeNetAddr to zero the sock addr, or
ask NSPR clients to zero the sock addr before passing it to
PR_InitializeNetAddr (essentially working around this bug)?
Status: NEW → ASSIGNED
Target Milestone: --- → 4.1.1

Comment 8

19 years ago
I think we should apply the fix.  It is extremely unlikely existing NSPR clients 
depend on current behavior.  Such dependence would be an incorrect use of the 
published API.

Assignee

Comment 11

19 years ago
I checked in my latest patch (id=23614)
on the tip, the NSPRPUB_RELEASE_4_1_BRANCH,
and the NSPRPUB_CLIENT_BRANCH.

I found that PR_StringToNetAddr should also
zero the sock addr first, but the code in
that function is more complicated.  If the
caller of PR_StringToNetAddr always passes
a PRNetAddr as they should, there is a simple
patch which I will attach next.
Assignee

Comment 12

19 years ago
On second thought, I'll open a separate bug
report for PR_StringToNetAddr so that I can
mark this bug fixed.
Status: ASSIGNED → RESOLVED
Closed: 19 years ago19 years ago
Resolution: --- → FIXED
Assignee

Comment 13

19 years ago
*** Bug 65549 has been marked as a duplicate of this bug. ***
Reporter

Comment 14

19 years ago
With this fix, I could build & run psm on NetBSD/i386-current.

BTW, two problems building psm.
While I'm building psm,
1)NSS didn't compile
I compiled it manually.
2)psm was installed to dist/NetBSD1.5Q_DBG.OBJ/bin.
Mozilla failed to run psm. Because mozilla launch dist/bin/start-psm.

Is it a NetBSD specific problem?

Anyway, this bug has fix with your work.
Thank you!
Assignee

Comment 15

19 years ago
Hi Taya,

Thanks for verifying that the fix works.

Unfortunately, I am not building PSM now, so
I won't be able to help you with the PSM/NSS
build problems.  I suggest that you post a
question to mozilla.crypto.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.