Closed Bug 1265093 Opened 8 years ago Closed 8 years ago

[CID 1358535][CID 1358648] Potential copy from 0x0 in AlignedBuffer::EnsureCapacity

Categories

(Core :: Audio/Video: Playback, defect)

48 Branch
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla48
Tracking Status
firefox48 --- fixed

People

(Reporter: mozbugz, Assigned: jya)

References

Details

(Whiteboard: CID1358535, CID1358648)

Attachments

(2 files)

In dom/media/MediaData.h:
> var_compare_op: Comparing this->mData to null implies that this->mData might be null.
> 229    if (mData && mCapacity >= sizeNeeded.value()) {
...
> CID 1358535 (#1 of 1): Dereference after null check (FORWARD_NULL)
> var_deref_model: Passing null pointer this->mData to PodCopy, which dereferences it.
> 244    PodCopy(newData, mData, mLength);
PodCopy eventually ends up in memcpy, and is the source is null, the behavior is undefined.
I'm guessing it's either checked, or bytes would be copied from 0x0, probably causing a read exception. So I don't think there is any security risk.

Assigning to jya, who first wrote this code in bug 1248861.
The only way mData is null is if mLength is 0. 

So it won't copy anything as PodCopy runs a loop with end condition being < aLength. 

This is a false positive. But I guess testing the condition would make it more clear
Thank you for looking into it. I've updated Coverity with this information.
You may close this bug if you like, or maybe add a comment and/or assert to the code?
Jya: are we closing this?
Flags: needinfo?(jyavenard)
It was a false positive, has mData can only be null if mLength is also 0.

Review commit: https://reviewboard.mozilla.org/r/47591/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/47591/
Attachment #8743138 - Flags: review?(gsquelart)
Attachment #8743139 - Flags: review?(gsquelart)
Flags: needinfo?(jyavenard)
Comment on attachment 8743138 [details]
MozReview Request: Bug 1265093: Silence CID 1358535. r?gerald

https://reviewboard.mozilla.org/r/47591/#review44409
Attachment #8743138 - Flags: review?(gsquelart) → review+
Comment on attachment 8743139 [details]
MozReview Request: Bug 1265093: Fix CID 1358648. r?gerald

https://reviewboard.mozilla.org/r/47603/#review44411
Attachment #8743139 - Flags: review?(gsquelart) → review+
CID 1358648:
> *** CID 1358648:  Uninitialized members  (UNINIT_CTOR)
> /obj-x86_64-pc-linux-gnu/dist/include/MediaInfo.h: 509 in mozilla::AudioConfig::ChannelLayout::ChannelLayout(unsigned int, const mozilla::AudioConfig::Channel *)()
> 505         ChannelLayout(uint32_t aChannels, const Channel* aConfig)
> 506         {
> 507           if (!aConfig) {
> 508             mValid = false;
>   CID 1358648:  Uninitialized members  (UNINIT_CTOR)
>   Non-static class member "mChannelMap" is not initialized in this constructor nor in any functions that it calls.
> 509             return;
> 510           }
> 511           mChannels.AppendElements(aConfig, aChannels);
> 512           UpdateChannelMap();
> 513         }
Summary: [CID 1358535] Potential copy from 0x0 in AlignedBuffer::EnsureCapacity → [CID 1358535][CID 1358648] Potential copy from 0x0 in AlignedBuffer::EnsureCapacity
Whiteboard: CID1358535 → CID1358535, CID1358648
You need to log in before you can comment on or make changes to this bug.