1087565 - (CVE-2011-3079) IPC Channel does not validate the listener.

Status: RESOLVED FIXED

(Core :: IPC, defect)

Description

[Jed Davis [:jld] ⟨⏰|UTC-7⟩ ⟦he/him⟧]

We have have probably inherited CVE-2011-3079 from our import of Chromium's IPC code 5½ years ago.

My understanding of this bug is: Chromium IPC endpoints have names, and knowing the name is sufficient to connect to it.  These names were predictable and, on Windows, are reflected in the named pipe namespace which an untrusted process can traverse.  The patch adds 32 bits of randomness to each name, and excludes that part from the pipe name on Windows.  (Anyone with more knowledge of IPC is invited to correct this summary if needed.)

There's also a patch on the bug which seems to not be directly related to the IPC issue itself, and has something to do with Windows security levels: https://crbug.com/117627#c18

+++ This bug was initially created as a clone of Bug #958895 +++

"IPC Channel does not validate the listener."
http://code.google.com/p/chromium/issues/detail?id=117627

Comment 1

[Ben Turner (not reading bugmail, use the needinfo flag!)]

But the attacker would still have to have a process running on the user's machine to do this, right?

Comment 2

[Jed Davis [:jld] ⟨⏰|UTC-7⟩ ⟦he/him⟧]

(In reply to ben turner [:bent] (use the needinfo? flag!) from comment #1)
> But the attacker would still have to have a process running on the user's
> machine to do this, right?

Yes.  The impact is privilege escalation from a compromised child process.

Comment 3

[Jed Davis [:jld] ⟨⏰|UTC-7⟩ ⟦he/him⟧]

I started trying to see how hard this is to backport, and it's definitely not easy (even by the standards of hand-applied diffs set by the other children of bug 958895).   IPC::Channel seems to have been nontrivially refactored between our fork and the upstream bug, at least for the Windows implementation, so I'm not even sure where some of the parts of the first patch should be applied without spending more time making sense of both copies.  Other parts of it depend on parts of chromium/base that don't (or no longer) exist in our tree, so they'd have to be (re-)imported or replaced, but that's a more straightforward complication.

Comment 4

[Bob Owen (:bobowen)]

I've been in and around this code recently and looking at their patches, I've convinced myself as to how to backport this.
So, I'll pick this one up.

Jed - from what I can tell this doesn't affect posix and their patches don't in effect change the posix implementation. So, I'm not sure this needs to block bug 866876.

Comment 5

[Jed Davis [:jld] ⟨⏰|UTC-7⟩ ⟦he/him⟧]

(In reply to Bob Owen (:bobowen) from comment #4)
> Jed - from what I can tell this doesn't affect posix and their patches don't
> in effect change the posix implementation. So, I'm not sure this needs to
> block bug 866876.

I've looked at the patches some more, and I think I agree — if I'm reading it correctly, the IPC hello message payload is just the pid on POSIX (and the fix here is to add a secret int for Windows only).  It's a little confusing because of the references to “validated channels” and needing to generate random names even in the POSIX case.  I'm still not as sure as I'd ideally like to be of what's going on with that.

Comment 6

[Bob Owen (:bobowen)]

Attachment: Verify the child process with a secret hello on Windows.

This is basically a port of [1] to our much older (modified) branch of the code.

I've had to change a few things to get it to work.

For the other changes mentioned in the bug:
[2] doesn't apply to our version of the code
[3] is a sandboxing code change, which we already have

I'm afraid I completely forgot about be careful pushing sec bugs to try ... sorry:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=0279d3d18835
https://treeherder.mozilla.org/#/jobs?repo=try&revision=bfb5eca57e57

* bobowen slaps his own wrists ... really hard


[1] https://chromium.googlesource.com/chromium/src/+/5c41e6e1cd446c3b2bc0ed17e66dffbc76f7c650%5E!/

[2] https://chromium.googlesource.com/chromium/src/+/0590878f77dd9d3791b936287db516a99d14efb0%5E!/

[3] https://chromium.googlesource.com/chromium/src/+/48fae61b8a6c9b741f001d478c595b6c7c0af4d9%5E!/

Comment 7

[David Anderson [:dvander] - inactive, e-mail if emergency]

Comment on attachment 8589804 [details] [diff] [review]
Verify the child process with a secret hello on Windows.

Review of attachment 8589804 [details] [diff] [review]:
-----------------------------------------------------------------

This looks fine, just some things that I'd like clarified or adjusted first.

::: ipc/chromium/src/chrome/common/ipc_channel.h
@@ +137,5 @@
> +  static std::wstring GenerateUniqueRandomChannelID();
> +
> +  // Generates a channel ID that, if passed to the client as a shared secret,
> +  // will validate that the client's authenticity. On platforms that do not
> +  // require additional this is simply calls GenerateUniqueRandomChannelID().

nit: "do not require additional validation"

::: ipc/chromium/src/chrome/common/ipc_channel_win.cc
@@ +164,5 @@
>    std::wostringstream ss;
> +  ss << L"\\\\.\\pipe\\chrome.";
> +
> +  // Prevent the shared secret from ending up in the pipe name.
> +  size_t index = channel_id.find_first_of(L'\\');

This strikes me as a little hacky. It would be nicer if GenerateVerifiedChannelID returned some kind of struct or tuple of (id, secret) or (channel-id, safe-pipe-id) instead, and Channel was constructed with that.

@@ +172,5 @@
> +    }
> +    ss << channel_id.substr(0, index - 1);
> +  } else {
> +    // This case is here to support predictable named pipes in tests.
> +    if (secret) {

CreatePipe() is the only caller of PipeName() and it passes a valid |secret|. Is there a reason to check for null here and above?

@@ +226,5 @@
> +  // Don't send the secret to the untrusted process, and don't send a secret
> +  // if the value is zero (for IPC backwards compatability).
> +  int32_t secret = validate_client_ ? 0 : client_secret_;
> +  if (!m->WriteInt(GetCurrentProcessId()) ||
> +      (secret && !m->WriteUInt32(secret))) {

nit: brace on newline for multi-line |if|

Using validate_client_ here is a little confusing. Reads better as:

  int32_t secret = (mode == MODE_CLIENT) ? client_secret_ : 0;

@@ +368,5 @@
> +          // The Hello message contains the process id and a secret if we need
> +          // to validate the client.
> +          MessageIterator it = MessageIterator(m);
> +          int32_t claimed_pid = it.NextInt();
> +          if (validate_client_ && (it.NextInt() != client_secret_)) {

Assuming we get rid of validate_client_ this check, I think, would be:

  if (mode == MODE_SERVER &&
      client_secret_ &&
      (it.NextInt() != client_secret_))
  {

::: ipc/chromium/src/chrome/common/ipc_channel_win.h
@@ +107,5 @@
>    // implement Unsound_NumQueuedMessages.
>    size_t output_queue_length_;
>  
> +  // Determines if we should validate a client's secret on connection.
> +  bool validate_client_;

It looks like we can drop this, since client_secret_ == 0 implies no validation. If for some reason we can't drop it, a name without "client" would be better (like "needs_validation_" or something).

@@ +112,5 @@
> +  // This is a unique per-channel value used to authenticate the client end of
> +  // a connection. If the value is non-zero, the client passes it in the hello
> +  // and the host validates. (We don't send the zero value fto preserve IPC
> +  // compatability with existing clients that don't validate the channel.)
> +  int32_t client_secret_;

The word "client" can get confusing in IPC. Would "shared_secret_" or "challenge_secret_" be better here?

Comment 8

[David Anderson [:dvander] - inactive, e-mail if emergency]

I assume the way the shared secret makes its way to the child process in the first place is a command line argument to CreateProcess(). If that's the case, we should check that unprivileged processes can't query that information with NtQueryInformationProcess(). Or as a precaution we could zap the PEB command line memory after it's been consumed.

Comment 9

[Bob Owen (:bobowen)]

Thanks for the review.

(In reply to David Anderson [:dvander] from comment #8)
> I assume the way the shared secret makes its way to the child process in the
> first place is a command line argument to CreateProcess(). If that's the
> case, we should check that unprivileged processes can't query that
> information with NtQueryInformationProcess(). Or as a precaution we could
> zap the PEB command line memory after it's been consumed.

Yeah, I wondered about that.
For what it's worth, I don't think that Chrome clears the command line memory at all and certainly you can still see the channel ID and secret in Process Explorer.

As another part of this bug fix they started using an untrusted integrity level on the renderers, I think to stop them from opening other low (or higher) integrity processes like their gpu process.
We can't use untrusted on the content process currently and I'm not clear what benefit that would give us at the moment anyway.
We use untrusted as the delayed integrity level for GMP processes.

I suppose the other risk (such as it is) is from other low privileged processes from using the pipe and impersonating a Gecko child process.
I'm not sure how easy it would be for them to determine and open a child process, steal the secret and then use their pipe.

I'm pretty new to all this IPC stuff, so it would be good for me to investigate it and see if clearing the secret or other measures would be useful.

I'm on PTO tomorrow and next week, so would you mind if I picked this up in a follow-up bug?
Although, I suspect I'm not going to get this landed before I go anyway. :)


(In reply to David Anderson [:dvander] from comment #7)
> Comment on attachment 8589804 [details] [diff] [review]
> Verify the child process with a secret hello on Windows.
> 
> Review of attachment 8589804 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> This looks fine, just some things that I'd like clarified or adjusted first.

In general I was trying to keep this as close to the chromium patch as possible, while not changing too much of our existing code.

Life is already made difficult by the fact that our chromium IPC code is so old and by the fact that we have modified it fairly heavily.
I was trying to not make matters worse, although maybe that's a lost cause. :-)

I've commented about the ones that aren't style / typos below.
If you still want the proposed changes I'll have to pick this up after my PTO.

> ::: ipc/chromium/src/chrome/common/ipc_channel_win.cc
> @@ +164,5 @@
> >    std::wostringstream ss;
> > +  ss << L"\\\\.\\pipe\\chrome.";
> > +
> > +  // Prevent the shared secret from ending up in the pipe name.
> > +  size_t index = channel_id.find_first_of(L'\\');
> 
> This strikes me as a little hacky. It would be nicer if
> GenerateVerifiedChannelID returned some kind of struct or tuple of (id,
> secret) or (channel-id, safe-pipe-id) instead, and Channel was constructed
> with that.

I agree, I imagine that they were trying to prevent the patch from hitting too many areas of their code, but I'm only guessing.
It does seem a bit odd given that in their patch they've already changed to use a ChannelHandle, so should have been able to add this without too much pain.

In our case we would have to change the other parts that use channel id and we'd end up moving this parsing logic to where we read the command line parameter.
 
> @@ +172,5 @@
> > +    }
> > +    ss << channel_id.substr(0, index - 1);
> > +  } else {
> > +    // This case is here to support predictable named pipes in tests.
> > +    if (secret) {
> 
> CreatePipe() is the only caller of PipeName() and it passes a valid
> |secret|. Is there a reason to check for null here and above?

There was another part of the code, that we don't have, that passes null.
Not entirely sure where they use that.

> @@ +226,5 @@
> > +  // Don't send the secret to the untrusted process, and don't send a secret
> > +  // if the value is zero (for IPC backwards compatability).
> > +  int32_t secret = validate_client_ ? 0 : client_secret_;
> > +  if (!m->WriteInt(GetCurrentProcessId()) ||
> > +      (secret && !m->WriteUInt32(secret))) {
> 
> nit: brace on newline for multi-line |if|
> 
> Using validate_client_ here is a little confusing. Reads better as:
> 
>   int32_t secret = (mode == MODE_CLIENT) ? client_secret_ : 0;

Yeah, I found this difficult to read, but I suppose it depends on whether the extra confusion of moving further away form their code is worth it.

I agree that this is clearer.

I think that other changes that didn't fit into our code would have made dropping validate_client_ altogether a little more difficult.

> @@ +112,5 @@
> > +  // This is a unique per-channel value used to authenticate the client end of
> > +  // a connection. If the value is non-zero, the client passes it in the hello
> > +  // and the host validates. (We don't send the zero value fto preserve IPC
> > +  // compatability with existing clients that don't validate the channel.)
> > +  int32_t client_secret_;
> 
> The word "client" can get confusing in IPC. Would "shared_secret_" or
> "challenge_secret_" be better here?

I suppose it is the client side of the pipe, but I agree this can be confusing.
Our use of Parent and Child has confused me a bit it the past as well. :-)

Comment 10

[David Anderson [:dvander] - inactive, e-mail if emergency]

(In reply to Bob Owen (:bobowen) (PTO 10-19 Apr) from comment #9)
> I'm on PTO tomorrow and next week, so would you mind if I picked this up in
> a follow-up bug?
> Although, I suspect I'm not going to get this landed before I go anyway. :)

Sure.

> In general I was trying to keep this as close to the chromium patch as
> possible, while not changing too much of our existing code.

Ah, I see. It doesn't bother me too much if we diverge from Chromium, since that ship sailed a long time ago. I would really like to see the word "client_" changed and validate_client_ removed if possible, the other stuff I'll leave up to you.

Comment 11

[David Anderson [:dvander] - inactive, e-mail if emergency]

Comment on attachment 8589804 [details] [diff] [review]
Verify the child process with a secret hello on Windows.

r=me conditional on follow-up fixes as noted

Comment 12

[Bob Owen (:bobowen)]

Attachment: Verify the child process with a secret hello on Windows. v2

Re-addressing your original review.
r?ing again as things didn't quite work out as hoped in some cases.

Try push as things have shifted a bit:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=9bc7385353bd

(In reply to David Anderson [:dvander] from comment #8)
> I assume the way the shared secret makes its way to the child process in the
> first place is a command line argument to CreateProcess(). If that's the
> case, we should check that unprivileged processes can't query that
> information with NtQueryInformationProcess(). Or as a precaution we could
> zap the PEB command line memory after it's been consumed.

Bug 1156835 filed as a follow-up.

(In reply to David Anderson [:dvander] from comment #7)
> Comment on attachment 8589804 [details] [diff] [review]

> > +  // require additional this is simply calls GenerateUniqueRandomChannelID().
> 
> nit: "do not require additional validation"

Fixed.

> ::: ipc/chromium/src/chrome/common/ipc_channel_win.cc
> @@ +164,5 @@
> >    std::wostringstream ss;
> > +  ss << L"\\\\.\\pipe\\chrome.";
> > +
> > +  // Prevent the shared secret from ending up in the pipe name.
> > +  size_t index = channel_id.find_first_of(L'\\');
> 
> This strikes me as a little hacky. It would be nicer if
> GenerateVerifiedChannelID returned some kind of struct or tuple of (id,
> secret) or (channel-id, safe-pipe-id) instead, and Channel was constructed
> with that.

I left this as we would need this logic somewhere when we parsed the channel from the command line.

> @@ +172,5 @@
> > +    }
> > +    ss << channel_id.substr(0, index - 1);
> > +  } else {
> > +    // This case is here to support predictable named pipes in tests.
> > +    if (secret) {
> 
> CreatePipe() is the only caller of PipeName() and it passes a valid
> |secret|. Is there a reason to check for null here and above?

Removed and added a MOZ_ASSERT at the top of the function for safety. 

> @@ +226,5 @@
> > +  // Don't send the secret to the untrusted process, and don't send a secret
> > +  // if the value is zero (for IPC backwards compatability).
> > +  int32_t secret = validate_client_ ? 0 : client_secret_;
> > +  if (!m->WriteInt(GetCurrentProcessId()) ||
> > +      (secret && !m->WriteUInt32(secret))) {
> 
> nit: brace on newline for multi-line |if|

Fixed.
Is this in our style guide?
I don't remember it, but I think it is definitely clearer with the multi-lines.
 
> Using validate_client_ here is a little confusing. Reads better as:
> 
>   int32_t secret = (mode == MODE_CLIENT) ? client_secret_ : 0;

Unfortunately mode isn't available here or in the other place where we'd need it.
I could have added it as a member variable, but that seemed like a bigger change.
So, I've renamed validate_client_ to waiting_for_shared_secret_ and changed the comments, to hopefully make it clear what is going on here.

> @@ +368,5 @@
> > +          // The Hello message contains the process id and a secret if we need
> > +          // to validate the client.
> > +          MessageIterator it = MessageIterator(m);
> > +          int32_t claimed_pid = it.NextInt();
> > +          if (validate_client_ && (it.NextInt() != client_secret_)) {
> 
> Assuming we get rid of validate_client_ this check, I think, would be:
> 
>   if (mode == MODE_SERVER &&
>       client_secret_ &&
>       (it.NextInt() != client_secret_))
>   {

As above mode isn't available here.
I also made the comment clearer here (hopefully).

> ::: ipc/chromium/src/chrome/common/ipc_channel_win.h
> @@ +107,5 @@
> >    // implement Unsound_NumQueuedMessages.
> >    size_t output_queue_length_;
> >  
> > +  // Determines if we should validate a client's secret on connection.
> > +  bool validate_client_;
> 
> It looks like we can drop this, since client_secret_ == 0 implies no
> validation. If for some reason we can't drop it, a name without "client"
> would be better (like "needs_validation_" or something).

This is now waiting_for_shared_secret_

> @@ +112,5 @@
> > +  // This is a unique per-channel value used to authenticate the client end of
> > +  // a connection. If the value is non-zero, the client passes it in the hello
> > +  // and the host validates. (We don't send the zero value fto preserve IPC
> > +  // compatability with existing clients that don't validate the channel.)
> > +  int32_t client_secret_;
> 
> The word "client" can get confusing in IPC. Would "shared_secret_" or
> "challenge_secret_" be better here?

This is now shared_secret_

Comment 13

[David Anderson [:dvander] - inactive, e-mail if emergency]

Comment on attachment 8595453 [details] [diff] [review]
Verify the child process with a secret hello on Windows. v2

Review of attachment 8595453 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good, thanks! re: style guide, not sure - it is for js/src. I haven't seen a situation where it looks worse.

::: ipc/chromium/src/chrome/common/ipc_channel_win.h
@@ +111,5 @@
>  
> +  // This is a unique per-channel value used to authenticate the client end of
> +  // a connection. If the value is non-zero, the client passes it in the hello
> +  // and the host validates. (We don't send the zero value to preserve IPC
> +  // compatability with existing clients that don't validate the channel.)

nit: compatibility

Comment 14

[Bob Owen (:bobowen)]

Comment on attachment 8595453 [details] [diff] [review]
Verify the child process with a secret hello on Windows. v2

[Security approval request comment]
How easily could an exploit be constructed based on the patch?
You would need to have a compromised process on the machine already, so that you could run arbitrary code.
Even then, the most you could do is "impersonate" a gecko child process using the named pipe.
For what it's worth, the Chromium guys only marked this as medium severity.

Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem?
Yes, the comments make the issue clear.
Also, this is already a public bug as it is reported for Chromium.

Which older supported branches are affected by this flaw?
Since the Chromium IPC code landed, I think, so all.
However, I think we only started using it for real, when either GMP or the thumbnail generation started to be used.

If not all supported branches, which bug introduced the flaw?
n/a

Do you have backports for the affected branches? If not, how different, hard to create, and risky will they be?
Doesn't quite apply cleanly on Beta/Aurora (because of one of my own changes :-) ), but would be really easy to fix, if it were deemed necessary.

How likely is this patch to cause regressions; how much testing does it need?
Should be pretty safe and if it does cause regressions, they should be fairly obvious and widespread for e10s on Windows I think.

Comment 15

[Al Billings [:abillings - ex-MoCo]]

Comment on attachment 8595453 [details] [diff] [review]
Verify the child process with a secret hello on Windows. v2

sec-approval+ for trunk.
We'll want Aurora, Beta, and ESR31 patches for this created and nominated as well.

Comment 16

[Bob Owen (:bobowen)]

remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/2cb85f5cadab

Thanks for the r+ and s-a+.

(In reply to David Anderson [:dvander] from comment #13)
> Comment on attachment 8595453 [details] [diff] [review]

> > +  // compatability with existing clients that don't validate the channel.)
> 
> nit: compatibility

Fixed before pushing, thanks.

Comment 17

[Bob Owen (:bobowen)]

Attachment: Aurora/Beta - Verify the child process with a secret hello on Windows.

Minor rebase due to some surrounding context changes.

Approval Request Comment
[Feature/regressing bug #]:
The code would have been brought in with the first cut of Chromium IPC code back in June 2009.

[User impact if declined]:
In theory a compromised, but low privileged process would be able to impersonate a Gecko child process, possible achieving some privilege escalation through communication with the firefox.exe process.

[Describe test coverage new/current, TreeHerder]:
Child processes are tested through GMP/EME, e10s and (I believe) thumbnail generation tests.

[Risks and why]: 
Medium-low - while not a trivial patch the idea behind it is simple. Any issues would hopefully be obvious as the code is involved with the IPC channel initialisation.

[String/UUID change made/needed]:
None.

Comment 18

[Bob Owen (:bobowen)]

Attachment: ESR31 - Verify the child process with a secret hello on Windows.

Again mainly surrounding context changes, but also an old version of ipc/chromium/src/base/atomic_sequence_num.h existed.
It looks like this was removed some time after ESR31, as it wasn't being used, so I've just overwritten it with the new version.

[Approval Request Comment]
If this is not a sec:{high,crit} bug, please state case for ESR consideration:
sec-high

User impact if declined:
In theory a compromised, but low privileged process would be able to impersonate a Gecko child process, possible achieving some privilege escalation through communication with the firefox.exe process.

Fix Landed on Version:
Fx40

Risk to taking this patch (and alternatives if risky):
Medium-low - while not a trivial patch the idea behind it is simple. Any issues would hopefully be obvious as the code is involved with the IPC channel initialisation.

String or UUID changes made by this patch: 
None

See https://wiki.mozilla.org/Release_Management/ESR_Landing_Process for more info.

Comment 19

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/2cb85f5cadab

Comment 20

[Jed Davis [:jld] ⟨⏰|UTC-7⟩ ⟦he/him⟧]

Is this a security issue for ESR31?  I didn't think we had any sandboxing on that branch, but maybe there are NPAPI plugins doing their own sandboxing that could be escaped this way?

Comment 21

[Bob Owen (:bobowen)]

(In reply to Jed Davis [:jld] {UTC-7} from comment #20)
> Is this a security issue for ESR31?  I didn't think we had any sandboxing on
> that branch, but maybe there are NPAPI plugins doing their own sandboxing
> that could be escaped this way?

Well, we have the thumbnail content process.
So, in theory a compromised non-gecko low privileged process could impersonate that process.
Like I've said before, I'm not sure how easy that would be in reality or whether it would be able to do anything damaging.

Comment 22

[Sylvestre Ledru [:Sylvestre]]

Comment on attachment 8596514 [details] [diff] [review]
Aurora/Beta - Verify the child process with a secret hello on Windows.

[Triage Comment]
Let's take it

Comment 23

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/78c576630fed

Comment 24

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-release/rev/c1f04200ed98

Comment 25

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/c1f04200ed98

Comment 26

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-esr31/rev/7f3854a8ddcf

Comment 27

[[SV Manager] Florin Mezei, QA (:FlorinMezei)]

Is there a need for manually testing this fix? If yes, could you please provide some detailed steps on how to do it?

Comment 28

[Bob Owen (:bobowen)]

(In reply to Florin Mezei, QA (:FlorinMezei) from comment #27)
> Is there a need for manually testing this fix? If yes, could you please
> provide some detailed steps on how to do it?

I'm not sure how you would go about testing this other than observing (using something like Process Explorer) that there is now a random secret number on the end of the channel ID in the command line.
If you managed to pause the child process (through debugging) before it connected to the named pipe and attempted to connect to the the named pipe (which is the channel ID without the extra "/<unique number>").
If you then don't send the correct Hello message (including the random secret) then it should fail.

In the Chromium bug, I think this was just a theoretical attack, it didn't have any PoC exploits.
The theory in the bug is that on Windows low privileged processes can enumerate named pipes and could potentially attempt to connect to them.

Comment 29

[Al Billings [:abillings - ex-MoCo]]

Who do I credit for this in our security advisory? You, Jed? Christoph? Both of you? Someone else? :-)

Comment 30

[[SV Manager] Florin Mezei, QA (:FlorinMezei)]

I was able to verify this on Windows 7 x64, in Firefox ESR 31.7.0 and 38.0, using Process Explorer. The plugin-container.exe process displays "\<random number>" appended at the end of the "--channel" parameter in the Command Line field. This appended value does not show in an older build like 31.2.0 ESR.

Comment 31

[Jed Davis [:jld] ⟨⏰|UTC-7⟩ ⟦he/him⟧]

(In reply to Al Billings [:abillings] from comment #29)
> Who do I credit for this in our security advisory? You, Jed? Christoph? Both
> of you? Someone else? :-)

I think it makes sense to credit both of us — cdiehl reported it to Mozilla originally, but the bug wasn't recognized as security-sensitive until I found it.

Comment 32

[Al Billings [:abillings - ex-MoCo]]

That seems fair. I'll do that.