Closed Bug 1328456 Opened 3 years ago Closed 3 years ago

disallow empty token names as the argument to nsIPK11TokenDB.findTokenByName

Categories

(Core :: Security: PSM, defect, P1)

defect

Tracking

()

RESOLVED FIXED
mozilla53
Tracking Status
firefox53 --- fixed

People

(Reporter: keeler, Assigned: keeler)

Details

(Keywords: addon-compat, Whiteboard: [psm-assigned])

Attachments

(1 file)

nsIPK11TokenDB.findTokenByName("") is shorthand for nsIPK11TokenDB.getInternalKeyToken() which is a little silly as it's not all that much shorter and it's a bit unexpected due to not being documented at all. We should just enforce that findTokenByName take a non-empty string and change existing code to call getInternalKeyToken.
Comment on attachment 8824218 [details]
bug 1328456 - disallow empty token names for nsIPK11TokenDB.findTokenByName

https://reviewboard.mozilla.org/r/102726/#review103486

Looks good.

::: security/manager/pki/resources/content/resetpassword.xul:11
(Diff revision 1)
>  <?xml-stylesheet href="chrome://global/skin/" type="text/css"?>
>  
>  <!DOCTYPE dialog SYSTEM "chrome://pippki/locale/pippki.dtd">
>  
>  <dialog id="reset_password" title="&resetPassword.title;"
>    xmlns="http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul"      

Would be nice to get rid of the trailing whitespace while you're here.

::: security/manager/ssl/nsIPK11TokenDB.idl:38
(Diff revision 1)
>    /*
> -   * Find a token by name
> +   * Find a token by name. Throws NS_ERROR_FAILURE if no token with the given
> +   * name can be found.
> +   * @param tokenName a string identifying the name of the token. Must be
> +   *                  non-empty.
> +   * @return nsIPK11Token a token with the given name

No harm in doing so, but we probably don't need to repeat the return type.
Attachment #8824218 - Flags: review?(cykesiopka.bmo) → review+
Pushed by dkeeler@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/f1b501863c27
disallow empty token names for nsIPK11TokenDB.findTokenByName r=Cykesiopka
Try looked good: https://treeherder.mozilla.org/#/jobs?repo=try&revision=7c0145e52eb6
Also, this could have an add-on compatibility impact. The fix should be fairly obvious, however (just use getInternalKeyToken).
Keywords: addon-compat
https://hg.mozilla.org/mozilla-central/rev/f1b501863c27
Status: NEW → RESOLVED
Closed: 3 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla53
You need to log in before you can comment on or make changes to this bug.