Closed Bug 1674332 Opened 4 years ago Closed 4 years ago

Fenix allows content processes to allocate lots of memory leading to Android closing other apps / being very slow

Categories

(Core :: JavaScript Engine, defect)

All
Android
defect

Tracking

()

RESOLVED INVALID

People

(Reporter: kirtikumar.a.r, Unassigned)

References

Details

(Keywords: csectype-oom, reporter-external, Whiteboard: [reporter-external] [client-bounty-form] [verif?])

Assigned to:- Firefox
Assigned by:- Kirtikumar Anandrao Ramchandani
Assigned on:- 30/10/2020
Vulnerable Product version and their behaviour:

  1. Firefox Lite- Application crash- (2.5.2)
  2. Firefox- Will close the other running applications and make device unresponsive for a few seconds. - (82.1.1 |Build #2015770923|)
  3. Firefox Focus- Same as Firefox (8.8.3)
  4. Firefox Nightly- Same as Firefox-Nightly 201028 17:00 (build #2015772363)
    Not sure what actually happens but it sounds like it is making the device run OOM or it's grabbing all CPU cycles it can. The other running apps were also closed (shouldn't happen, due to sandboxing, though).
    Device: Xiaomi Redmi Note 5 Pro (OS-9.0 P | Ram5739MB)
    Video PoC: https://drive.google.com/file/d/1YggZsnagcJo9BnunZeQQZ2aIBS30NZ1V/view?usp=drivesdk
    PoC: https://kirtikumarar.com/Firefox_OOM.html
Flags: sec-bounty?

(In reply to Kirtikumar Anandrao Ramchandani from comment #0)

PoC: https://kirtikumarar.com/Firefox_OOM.html

This contains:

<script>
const MB = 1024 * 1024,
      block_size = 32 * MB;
array = Array(block_size).fill(1.1);
array.prop = 1;
args = Array(2048 * MB / block_size - 2).fill(array);
args.push(Array(block_size));
array.concat.apply(array, args);
</script>

Can you clarify why you think this causes an "integer overflow"? The results from comment #0 do not seem to indicate this.

Group: firefox-core-security → mobile-core-security
Type: task → defect
Component: Security → Security: Android
Flags: needinfo?(kirtikumar.a.r)
Product: Firefox → Fenix

The exploit forces the method to return an array whose length value is bigger than the actual size of the backing store, which is essentially a ready-to-use OOB read/write exploitation primitive.

Flags: needinfo?(kirtikumar.a.r)

The exploit is declaring a constant value for MB, and a const value for block_size, creates an array of size 32 * (1024 * 1024) and fills the array with a static value of 1.1. it looks like it will generate some very large number which could touch the value cap.

I admit I'm not an expert on JS engine exploits - I'm just trying to direct this report to the right people - but this doesn't really make sense to me.

I tried the PoC on desktop, and it allocates some 2-3GB of stuff (during which the user gets an opportunity to end the content process) and then GCs it all. That makes sense to me considering what the PoC does.

(In reply to Kirtikumar Anandrao Ramchandani from comment #2)

The exploit forces the method

which method?

to return an array whose length value is bigger than the actual size of the backing store,

What is the "backing store" in this explanation? And which array are we talking about? The result of the concat call? Something else?

(In reply to Kirtikumar Anandrao Ramchandani from comment #3)

The exploit is declaring a constant value for MB, and a const value for block_size, creates an array of size 32 * (1024 * 1024) and fills the array with a static value of 1.1.

OK, so now you have an array of some 33 million items, all of which are set to 1.1 .

it looks like it will generate some very large number

It's an array, not a number, and none of the code tries to use it in a different way, AFAICT.

You then create the array args which is just an array of length 62, all of whose values are set to the 33m array. Then you append another empty 33m item array into args, and then you concat all 62+1+1 items together into a single array, which then gets GC'd as it isn't stored or used.

which could touch the value cap.

What does this refer to?

Basically, to me, the PoC and your claim this is a "ready to use OOB read / write primitive" suggest that you think there's a JS engine issue, but if there was, I'd expect it to reproduce on desktop too.

The behaviour described in comment #0 instead reads like the devices are all OOM'ing, and then android nukes us from orbit. That makes sense and is a dos-style attack, but isn't really a JS engine issue.

Can you clarify what you believe to be the issue here?

Flags: needinfo?(kirtikumar.a.r)

(Note: edited for formatting; ~ gijs )

(In reply to :Gijs (he/him) from comment #4)

I tried the PoC on desktop, and it allocates some 2-3GB of stuff (during which the user gets an opportunity to end the content process) and then GCs it all. That makes sense to me considering what the PoC does.

IIRC the PoC would have made the device unresponsive too for some duration of time due to that, other applications won't work smoothly.

(In reply to Kirtikumar Anandrao Ramchandani from comment #2)

to return an array whose length value is bigger than the actual size of the backing store,
What is the "backing store" in this explanation? And which array are we talking about? The result of the concat call? Something else?

The array that the function writes the result into is backing store. The backing store here it means memory. Or maybe you can say the backing store is referring to actual size of buffer let's say length value is bigger than the actual size of the buffer. E.g.: Let's say it allocate certain memory 1024, but you force the method to return a value which more than 1024 memory. And it crashes the application. If the crash will overwrite the EIP then we can control the EIP which will turn into code execution.
E.g. char buffer[100];
memset(buffer, 0xff, 200);
buffer is the backing store for the memset call.

It's an array, not a number, and none of the code tries to use it in a different way, AFAICT.

Sorry. Yes, it's an array, I wrote a number by mistake.

Basically, to me, the PoC and your claim this is a "ready to use OOB read / write primitive" suggest that you think there's a JS engine issue, but if there was, I'd expect it to reproduce on desktop too.

The behaviour described in comment #0 instead reads like the devices are all OOM'ing, and then android nukes us from orbit. That makes sense and is a dos-style attack, but isn't really a JS engine issue.

No, it looks like the issue lies in Firefox and not the device. Reason: I tried the exploit on another browser and, it was crashed too and got log entry:

10-29 15:29:15.441 10558 10596 W cr_ChildProcessConn: onServiceDisconnected (crash or killed by oom): pid=10609 bindings:W S state:3 counts:0,0,0,1,
10-29 15:29:15.453 10558 10558 : [ERROR:aw_browser_terminator.cc(123)] Renderer process (10609) crash detected (code 6).
10-29 15:29:15.456 10558 10558 : [FATAL:crashpad_client_linux.cc(666)] Render process (10609)'s crash wasn't handled by all associated  webviews, triggering application crash.
10-29 15:29:15.456 10558 10558 : Fatal signal 5 (SIGTRAP), code -6 (SI_TKILL) in tid 10558 , pid 10558

Can you clarify what you believe to be the issue here?

The issue looks like to be in present in the browser and not the device.

Flags: needinfo?(kirtikumar.a.r)

The crash report of another browser can be seen here: https://drive.google.com/file/d/1ZGCu1rswQl7ZzVim5ZIb20DT2ULezT_5/view?usp=sharing ( Haven't disclosed the browser name). It shows crash during the renderer process.

Please open a new issue for Firefox Lite at https://bugzilla.mozilla.org/enter_bug.cgi?product=Emerging%20Markets&component=Security%3A%20%20Firefox%20Lite they are completely separate products from Fenix. Since it is a downstream consumer of the system webview I doubt there is much we can do about bad content but it is worth a look.

Thank you! Creating another issue now.

Hello,
I have created the issue the Bug ID is 1674451. Cheers!

Group: mobile-core-security → core-security
Crash Signature: [@ OOM | large | NS_ABORT_OOM | txMozillaXMLOutput::characters ]
Component: Security: Android → XSLT
Product: Fenix → Core
Status: UNCONFIRMED → NEW
Ever confirmed: true
OS: Unspecified → Android
Hardware: Unspecified → All
Version: unspecified → Trunk
Crash Signature: [@ OOM | large | NS_ABORT_OOM | txMozillaXMLOutput::characters ]

Short of the JS engine allocating less memory/using less cpu there is not anything we can do from the Fenix side to stop the OS from killing the app because it is exceeding OS limits on memory and cpu time.

Component: XSLT → JavaScript Engine

Respected sir,
Firstly, apologies for the inconvenience caused. The issue isn't of the operating system. The issue actually lies in Firefox browser you can test on other browsers and verify the behaviour.
For the XSLT, I have created separate submission: 1674503 with some description in it. And it isn't compulsory that OOM can't go beyond a DoS. What if there's control over EIP which will turn an issue into code execution.

Group: core-security → javascript-core-security

(In reply to Kirtikumar Anandrao Ramchandani from comment #18)

And it isn't compulsory that OOM can't go beyond a DoS. What if there's control over EIP which will turn an issue into code execution.

You've said this a few times now but a process crash is a crash, I'm not aware of any way for that to change EIP somehow and execution to continue instead of the process being killed.

It's definitely possible we're missing a GC trigger (although it's also possible the allocated data is just too big), but this looks like a non-exploitable OOM/DOS so far.

Apologies. Correct, it looks like it won't go beyond a DoS because in a OOM, we simply exhaust the process's max. available memory. Haven't tried to check further into this issue yet.

See Also: → 1674451
Group: javascript-core-security
Keywords: csectype-oom
See Also: → 1674331

Is this a bug? What should we be doing differently? It seems like if a web page tries to use unreasonable amounts of memory, it should be stopped at some point... and that is what is happening.

For now, I'll close this with RESOLVED INVALID, indicating that it's not really a bug and there's nothing to do here. If we really should be doing something differently, please reopen it or file a new one.

Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → INVALID

Marking bounty- unless this can be shown to be an actual vulnerability vs a way to cause an OOM.

Flags: sec-bounty? → sec-bounty-
You need to log in before you can comment on or make changes to this bug.