Closed Bug 338179 Opened 14 years ago Closed 14 years ago

Ineffective NULL checks in nsSocket::Send, nsSocket::Recv (xpinstall/wizard/libxpnet/src/nsSocket.cpp)

Categories

(SeaMonkey :: Installer, defect, minor)

defect
Not set
minor

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: kherron+mozilla, Assigned: ehsan)

References

()

Details

(Keywords: coverity, Whiteboard: [good first bug])

Attachments

(1 file, 1 obsolete file)

This is coverity CIDs 237 and 238. Please refer to the sample URL.

|nsSocket::Send| (line 291) and nsSocket::Recv| (line 358) both check their arguments and return E_PARAM on an error. But the checks ignore null |aBufSize|. Both functions unconditionally dereference |aBufSize| later.
Whiteboard: [good first bug]
Attached patch Patch to add checks for aBufSize (obsolete) — Splinter Review
Is this it? It's a 2 line patch to add checks for aBufSize.
Attachment #224809 - Flags: review?(dveditz)
Comment on attachment 224809 [details] [diff] [review]
Patch to add checks for aBufSize

>+    if (!aBuf || !aBufSize || (aBufSize && (*aBufSize <= 0)) || mFd < 0)

If the !aBufSize test fails, you don't need to check aBufSize again
!aBufSize || (*aBufSize <= 0)
Attachment #224809 - Flags: review?(dveditz) → review-
Doing the obvious...  :-)
Assignee: nobody → ehsan.akhgari
Attachment #224809 - Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #235874 - Flags: superreview?(ajschult)
Attachment #235874 - Flags: review?(ajschult)
Comment on attachment 235874 [details] [diff] [review]
Patch to add checks for aBufSize (revised)

I'm not an SR, but this patch is sufficiently trivial.
Attachment #235874 - Flags: superreview?(ajschult)
Attachment #235874 - Flags: review?(ajschult)
Attachment #235874 - Flags: review+
landed on trunk.
thanks, Ehsan
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Version: unspecified → Trunk
Keywords: qawanted
Keywords: qawanted
Keywords: verifyme
Keywords: verifyme
You need to log in before you can comment on or make changes to this bug.