Closed
Bug 1427171
Opened 7 years ago
Closed 2 years ago
[Static Analysis] Dereference null return value nsAccessibilityService::CreateAccessible
Categories
(Core :: Disability Access APIs, enhancement)
Core
Disability Access APIs
Tracking
()
RESOLVED
WORKSFORME
Tracking | Status | |
---|---|---|
firefox59 | --- | affected |
People
(Reporter: andi, Assigned: andi)
References
(Blocks 1 open bug)
Details
(Keywords: coverity, Whiteboard: CID 1426941)
Attachments
(1 file)
(deleted),
text/x-review-board-request
|
Details |
The Static Analysis tool Coverity detected that a return null pointer dereference occurs in several places where return pointer of |aContext->ARIARoleMap| is passed around and later dereferenced like:
>> if (!roleMapEntry && newAcc && aContext->HasStrongARIARole()) {
>> if (frame->AccessibleType() == eHTMLTableRowType) {
>> const nsRoleMapEntry* contextRoleMap = aContext->ARIARoleMap();
>> if (!contextRoleMap->IsOfType(eTable))
>> roleMapEntry = &aria::gEmptyRoleMap;
Looking through code this should be guarded of null pointer dereference like:
>>inline bool
>>Accessible::IsSearchbox() const
>>{
>> const nsRoleMapEntry* roleMapEntry = ARIARoleMap();
>> return (roleMapEntry && roleMapEntry->Is(nsGkAtoms::searchbox)) ||
>> (mContent->IsHTMLElement(nsGkAtoms::input) &&
>> mContent->AsElement()->AttrValueIs(kNameSpaceID_None, nsGkAtoms::type,
>> nsGkAtoms::search, eCaseMatters));
>>}
Comment hidden (mozreview-request) |
Comment 2•7 years ago
|
||
Comment on attachment 8938932 [details]
Bug 1427171 - prevent null pointer dereference when using return pointer from aContext->ARIARoleMap().
Alex would you want some kind of assert here?
Attachment #8938932 -
Flags: review?(dbolter) → review?(surkov.alexander)
Comment 3•7 years ago
|
||
mozreview-review |
Comment on attachment 8938932 [details]
Bug 1427171 - prevent null pointer dereference when using return pointer from aContext->ARIARoleMap().
https://reviewboard.mozilla.org/r/209396/#review215408
::: accessible/base/nsAccessibilityService.cpp:1203
(Diff revision 1)
> // If table has strong ARIA role then all table descendants shouldn't
> // expose their native roles.
> if (!roleMapEntry && newAcc && aContext->HasStrongARIARole()) {
> if (frame->AccessibleType() == eHTMLTableRowType) {
> const nsRoleMapEntry* contextRoleMap = aContext->ARIARoleMap();
> - if (!contextRoleMap->IsOfType(eTable))
> + if (contextRoleMap && !contextRoleMap->IsOfType(eTable))
HasStrongARIARole() guarantees us that aContext->ARIARoleMap() is never null. It appears that the static analysys gave a false positive in this case.
Not sure what is the best way to proceed, either leave the code untouched or make it more straightforward to avoid possible misreadings.
Attachment #8938932 -
Flags: review?(surkov.alexander)
Assignee | ||
Updated•2 years ago
|
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → WORKSFORME
You need to log in
before you can comment on or make changes to this bug.
Description
•