[Static Analysis][INT Overflow] Function LookupBestMatch from SurfaceCache.cpp overflow on idealArea

RESOLVED FIXED in Firefox 46

Status

()

Core
ImageLib
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: andi, Assigned: andi)

Tracking

({coverity})

Trunk
mozilla47
coverity
Points:
---

Firefox Tracking Flags

(firefox45 affected, firefox46 fixed, firefox47 fixed)

Details

(Whiteboard: CID 1340239)

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(2 attachments)

(Assignee)

Description

2 years ago
The Static Analysis tool Scan-Build added that variable idealArea will be assigned a potentially overflow value from int32.
(Assignee)

Comment 1

2 years ago
Created attachment 8692503 [details] [diff] [review]
Bug 1228314.diff

Hello Seth could you please take a look other this issue and tell me if the patch seems ok?
Flags: needinfo?(seth)
(Assignee)

Updated

2 years ago
Whiteboard: CID 1340239
(Assignee)

Comment 2

2 years ago
A correction to my first post the error was generated by Coverity not Scan-build
(Assignee)

Comment 3

2 years ago
Created attachment 8709049 [details]
MozReview Request: Bug 1228314 - added static_cast<int64>  in order to avoid overflow. r?seth

Review commit: https://reviewboard.mozilla.org/r/31241/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/31241/
Attachment #8709049 - Flags: review?(seth)
(Assignee)

Updated

2 years ago
Flags: needinfo?(seth)
(Assignee)

Updated

2 years ago
Attachment #8709049 - Flags: review?(netzen)
Comment on attachment 8709049 [details]
MozReview Request: Bug 1228314 - added static_cast<int64>  in order to avoid overflow. r?seth

Sorry, I am not ideal for this review.
Attachment #8709049 - Flags: review?(netzen)
Attachment #8709049 - Flags: review?(seth) → review+
Comment on attachment 8709049 [details]
MozReview Request: Bug 1228314 - added static_cast<int64>  in order to avoid overflow. r?seth

https://reviewboard.mozilla.org/r/31241/#review33283

Looks good! Thanks for identifying this issue.

::: image/SurfaceCache.cpp:330
(Diff revision 1)
> -      int64_t idealArea = idealKey.Size().width * idealKey.Size().height;
> +      int64_t idealArea = static_cast<int64_t>(idealKey.Size().width) * static_cast<int64_t>(idealKey.Size().height);

This looks over 80 columns. Please insert a line break right after the '*'.
(Assignee)

Comment 6

2 years ago
Comment on attachment 8709049 [details]
MozReview Request: Bug 1228314 - added static_cast<int64>  in order to avoid overflow. r?seth

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/31241/diff/1-2/

Comment 7

2 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/9727cdebb2ee

Comment 8

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/9727cdebb2ee
Status: NEW → RESOLVED
Last Resolved: 2 years ago
status-firefox47: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla47
Depends on: 1251742
Comment on attachment 8709049 [details]
MozReview Request: Bug 1228314 - added static_cast<int64>  in order to avoid overflow. r?seth

Need this for bug 1251091.

Approval Request Comment
[Feature/regressing bug #]: bug 1119774 added this code
[User impact if declined]: needed for bug 1251091
[Describe test coverage new/current, TreeHerder]: none
[Risks and why]: safe
[String/UUID change made/needed]: none
Attachment #8709049 - Flags: approval-mozilla-aurora?
Marking 46 affected.
status-firefox46: --- → affected
Comment on attachment 8709049 [details]
MozReview Request: Bug 1228314 - added static_cast<int64>  in order to avoid overflow. r?seth

We need this for the other related fixes for surface issues
Please uplift to aurora.
Attachment #8709049 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+

Comment 12

2 years ago
bugherderuplift
https://hg.mozilla.org/releases/mozilla-aurora/rev/c9563ef17e18
status-firefox46: affected → fixed
You need to log in before you can comment on or make changes to this bug.