1570457 - Get wininet.dll out of the content process

Status: RESOLVED FIXED

(Core :: Security: Process Sandboxing, task)

Description

[Tom Ritter [:tjr]]

To obtain the local IP address, which is used by WebRTC, the content process calls InternetGetConnectedStateExW which will load wininet.dll lazily.

But after we've lowered ourselves to a restricted token, we can't load dlls lazily (we crash), so we need to load this one before we lower.

I tested by sticking a LoadLibrary at the top of LowerToken and confirmed that doing so does in fact fix the crash.

Comment 1

[Tom Ritter [:tjr]]

Bob is this what we want to be doing? (Or should this call be remoted? I presume that loading this dll will not allow internet connections even if we are otherwise a fully restricted token...)

If it is what we want to do; should we push libraries-to-early-load from the Parent like we do for GMP; or can we just hardcode them?

I think the GMP complexity is because we don't always need to load all of them; but in this case (like RDD) we probably do just want to load it unconditionally.

Comment 2

[(No longer employed by Mozilla) Aaron Klotz]

It's Bob's call, but honestly I'm more inclined to remote this.

Comment 3

[BugBot [:suhaib / :marco/ :calixte]]

Bugbug thinks this bug is a task, but please change it back in case of error.

Comment 4

[Bob Owen (:bobowen)]

(In reply to Tom Ritter [:tjr] from comment #1)

Bob is this what we want to be doing? (Or should this call be remoted? I presume that loading this dll will not allow internet connections even if we are otherwise a fully restricted token...)

I hit something similar, when I was looking at this a few weeks ago, I agree with aklotz I would think remoting is the preferred option (given that eventually I assume we'd like the call to fail in the content process even if the DLL is loaded). This could be a short term option if we really need to (until we actually block the call, assuming USER_RESTRICTED doesn't already do this).
I thought the thing I saw was in the depths of webrtc code (but I got distracted by other things and I suspect this is the same), so I thought that it might not be too easy to remote, but this looks like it would be OK.
Not sure how easy it'll be to make it async, I suppose it depends on what is higher up the stack.

Over internet connections being blocked ... I know that USER_RESTRICTED blocked the piece of test code jimm gave me a while ago and I don't think it was just to do with DLL loading.
It's a little difficult to be definitive on Windows as to whether it blocks all types of connections.

If it is what we want to do; should we push libraries-to-early-load from the Parent like we do for GMP; or can we just hardcode them?

If we did go down the route of pre-loading DLLs, I don't immediately see why we would need to push this from the parent.

Comment 5

[Gian-Carlo Pascutto [:gcp]]

Bob, is this resolved due to the socket process?

Comment 6

[Bob Owen (:bobowen)]

(In reply to Gian-Carlo Pascutto [:gcp] from comment #5)

Bob, is this resolved due to the socket process?

As far as I can tell this is happening in the main process now on Nightly and even though the socket process isn't on release yet, I don't see wininet.dll loaded in any of the content processes after running some webrtc tests on Release.
I think that this was probably fixed by bug 1567892 that this bug's dependency was duped to.

bwc - would you mind confirming if the above is correct?

Comment 7

[Byron Campen [:bwc]]

Yes, bug 1567892 seems to have taken care of this. Not sure if there is anything else using this dll though.

Comment 8

[Bob Owen (:bobowen)]

(In reply to Byron Campen [:bwc] from comment #7)

Yes, bug 1567892 seems to have taken care of this. Not sure if there is anything else using this dll though.

Thanks, I think we're reasonably confident that this was the only thing using it, so I'll resolve.