1882344 - Website can obtain a wake lock that won't be released

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[Vincent Hilla [:vhilla]]

I'm not sure if this is a legitimate security issue, but Bug 1881070 shows a way for websites to obtain a screen wake lock that will not be released until the window closes. A website can prevent a device from turning off the screen or going to sleep even if it is not visible or active.

Example

window.addEventListener('load', async () => {
    iframe = document.createElement('iframe')
    document.documentElement.appendChild(iframe)
    lock = iframe.contentWindow.navigator.wakeLock.request()
}

Explanation

What's happening is that

• iframe loads the initial about:blank, document A
• invoking navigator.wakeLock creates a WakeLockJS object that is associated with the inner window and attaches activity, visibility listeners to document A
• WakeLockJS::Request schedules WakeLockJS::Obtain in a runnable for obtaining a lock
• iframe loads the target document B and due to this special case, the document is replaced while the inner window stays the same
• WakeLockJS gets notified of document A changing activity. But it doesn't get notified of the document being changed, so does not update the listeners
• Obtain creates a lock for document B
• If document B becomes inactive or hidden, the lock won't be released as WakeLockJS won't be notified.

WakeLockJS needs to either be notified of the document being replaced, or the document has to UnlockAllWakeLocks on becoming inactive or hidden.

Note that this also happens if we create the WakeLockJS object immediately, but request a lock later. Still, to follow the spec, Obtain should create a lock for the document at the time Request was called.

Comment 1

[Vincent Hilla [:vhilla]]

Attachment: Bug 1882344 - Have Document instead of WakeLockJS release locks on loss of activity. r=edgar,#dom-core

Comment 2

[Andrew McCreight [:mccr8]]

I appreciate the caution, but I think this doesn't really need to be a security issue. More of an annoyance than anything. I assume a user can still manually lock the screen or put the device to sleep.

Comment 4

[Vincent Hilla [:vhilla]]

Comment on attachment 9387880 [details]
Bug 1882344 - Have Document instead of WakeLockJS release locks on loss of activity. r=edgar,#dom-core

Beta/Release Uplift Approval Request

• User impact if declined: We are concerned that some wake locks might fail to be released. This would cause user devices to not turn off the display automatically.
• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: No
• Needs manual test from QE?: No
• If yes, steps to reproduce:
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): Not risky, as we merely stop the API in early beta from riding the train to release
• String changes made/needed:
• Is Android affected?: Yes

Comment 5

[Dianna Smith [:diannaS]]

:vhilla is this patch dependent uplifting the disabling Screen Wake Lock API on late beta/release in bug 1883724?

Comment 6

[Vincent Hilla [:vhilla]]

Comment on attachment 9387880 [details]
Bug 1882344 - Have Document instead of WakeLockJS release locks on loss of activity. r=edgar,#dom-core

Oh, I'm sorry I wanted to request uplift for Bug 1883724

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/a293e6927cab
Have Document instead of WakeLockJS release locks on loss of activity. r=edgar,dom-core

Comment 8

[Iulian Moraru]

Backed out for causing multiple failures.

• Push with failures

• Failure log for mochitest-plain failures

• Failure log for build bustages

• Backout link

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/8130451713b9
Have Document instead of WakeLockJS release locks on loss of activity. r=edgar,dom-core

Comment 10

[Sandor Molnar[:smolnar]]

https://hg.mozilla.org/mozilla-central/rev/8130451713b9