Add linter rule to block calling Principal->GetURI()
Categories
(Developer Infrastructure :: Source Code Analysis, task, P2)
Tracking
(firefox75 fixed)
Tracking | Status | |
---|---|---|
firefox75 | --- | fixed |
People
(Reporter: ckerschb, Assigned: sstreich)
References
(Blocks 1 open bug)
Details
(Whiteboard: [domsecurity-active])
Attachments
(1 file)
Within Bug we would like refactor the Principal API so to ask the Principal security questions but do not query the URI of a principal and make security decisions based on that URI.
Wile Bug 1577165 is on the way we should somehow ensure to not introduce new calls to principal->GetURI.
Reporter | ||
Comment 1•6 years ago
|
||
Sylvestre, where would I add such a rule? Here is what we would like to prevent being called:
https://searchfox.org/mozilla-central/rev/e878e5b81bb319c141900ce9cfcde732df5c8449/caps/nsIPrincipal.idl#78
Comment 2•6 years ago
|
||
So, when the class of the object if nsIPrincipal (or a child) and call GetURI, we want to break the CI ?
Is there a backlog of this pattern in the code base?
Reporter | ||
Comment 3•6 years ago
•
|
||
(In reply to Sylvestre Ledru [:Sylvestre] from comment #2)
Is there a backlog of this pattern in the code base?
Yes, there are a lot of callsites left currently in the codebase, see:
https://searchfox.org/mozilla-central/search?q=symbol:_ZN12nsIPrincipal6GetURIEPP6nsIURI&redirect=false
I think it's more than just breaking the CI, right?
Comment 4•6 years ago
•
|
||
So, we want to warn for new usage at review phase but not deal with previous cases (just like with clang-tidy), right?
Reporter | ||
Comment 5•6 years ago
|
||
(In reply to Sylvestre Ledru [:Sylvestre] from comment #4)
So, we want to warn for new usage at review phase but not deal with previous cases (just like with clang-tidy), right?
Yes, that is correct! Is that possible? If so, can you help me set that up?
Comment 6•6 years ago
|
||
Yeah, Andi will explain how it works (or do it for you)
Comment 7•6 years ago
|
||
In order to have this as a checker we need to develop a matcher that would match CallExpr
and fro the CallExpr
we will get the CXXMethodDecl
and determine if it's parent is QualType
- Principal
.
Now I see myself needed to ask how many occurrences of this kind we have in the code and If we want to change them before landing the checker or not. This matters on the way how we want to post clang-diagnostic-message
, if we push the checker in tree and we consider the diagnostic to be error
and we don't fix the issues prior than we will bust the build otherwise we are fine. But if we want to push the checker but we don't have time or willingness to fix the in-tree issues by the time we push we should emit warnings
as clang-diagnostic-message
.
Please note no matter what we emit these issues will be detected during review-phase
by the bot.
Reporter | ||
Updated•6 years ago
|
Assignee | ||
Comment 8•6 years ago
|
||
Comment 10•5 years ago
|
||
bugherder |
Updated•3 years ago
|
Description
•