memset 0 of the entire PRNetAddr in PR_InitializeNetAddr

RESOLVED FIXED in 4.15

Status

RESOLVED FIXED
2 years ago
a year ago

People

(Reporter: schien, Assigned: schien)

Tracking

other
4.15

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

PR_InitializeNetAddr only memset 0 for ipv4 struct. We should clear the entire PRNetAddr for supporting ipv6 as well.

See bug 1326483 comment #24 for the detail.
Comment hidden (mozreview-request)
I confirm that this fixes the problem I found in bug 1333185.

BTW I think that PR_InitializeNetAddr() needs the IP address family as an input, because even though this properly memset's the struct now it still then does everything based on the AF_INET (even when that is the wrong address family). But that is probably for another ticket to fix.
Blocks: 1333185

Comment 3

2 years ago
mozreview-review
Comment on attachment 8826072 [details]
Bug 1330490 - clear entire PRNetAddr struct in PR_InitializeNetAddr for IPv6.

https://reviewboard.mozilla.org/r/104122/#review110708

::: nsprpub/pr/src/misc/prnetdb.c:1408
(Diff revision 1)
>      PRNetAddrValue val, PRUint16 port, PRNetAddr *addr)
>  {
>      PRStatus rv = PR_SUCCESS;
>      if (!_pr_initialized) _PR_ImplicitInitialization();
>  
> -	if (val != PR_IpAddrNull) memset(addr, 0, sizeof(addr->inet));
> +	if (val != PR_IpAddrNull) memset(addr, 0, sizeof(*addr));

I don't *think* this should cause any problems, but I was looking at the history here, and there are some comments on bug 54796 that are a little concerning. I'm not sure they're still relevant nowadays, though.
Attachment #8826072 - Flags: review?(ted) → review+
I double checked the current mozilla-central and doesn't found any code that passes sockaddr_in to PR_InitializedNetAddr.

I'm not sure about the proper way to move this bug forward since NSPR is a shared library to many project. Do I simply land this bug via mozreview?
Flags: needinfo?(ted)
No, this needs to land in the NSPR repository.
Flags: needinfo?(ted)
Keywords: checkin-needed
Created attachment 8836592 [details] [diff] [review]
[patch for nspr repo] bug1330490.patch

Need someone to land this patch on nspr repo. @ted can you help?
Flags: needinfo?(ted)
Attachment #8836592 - Flags: checked-in?
@kaie can you help merge this patch to NSPR repo?
Flags: needinfo?(kaie)

Comment 8

2 years ago
https://hg.mozilla.org/projects/nspr/rev/2f1bfbcc79baac751f8bd83a9f799f33366b8d3a
Status: NEW → RESOLVED
Last Resolved: 2 years ago
Flags: needinfo?(ted)
Flags: needinfo?(kaie)
Resolution: --- → FIXED
Target Milestone: --- → 4.14
Attachment #8836592 - Flags: checked-in?
Keywords: checkin-needed

Updated

2 years ago
Target Milestone: 4.14 → 4.15
Attachment #8826072 - Attachment is obsolete: true
You need to log in before you can comment on or make changes to this bug.