Potential DOS of Mobile Applications because of no null check in ImageRequest.java [bug]

[knadt]

#2501 # Description

I have encountered multiple android applications that crash and sometimes become unusable (Denial of Service) for the user, when the application accepts markdown. to trigger this exploit a malicious user would send command ![Picture Title](file://127.0.0.1/doesnotexist.jpg) to a user in a chat. When the message gets rendered on the users phone the application cashes for and sometimes continues to crash until the exploit is removed. In some applications that allows chats, the chat might open after reopening the application cause a Denial of Service on the valid user. Sometimes it is not possible to remove the malicious markdown. I believe the code south be validated and check for a null value and caught if found.

Reproduction

on line 393 of ImageRequest.java there is a missing null check. By utilizing the below code, the application crashes with the below error.

Uncaught Error: Attempt to invoke virtual method 
'int java.lang.String.lastIndexOf(int)' on a null object reference

Therefore if an application passes user-supplied data to the getSize() method an error will be thrown.

import React from 'react';
const imageSize = Image.getSize('file:',_=>{} );

snack.expo.io source code

The getSize method is not the only part affected. If any React Native call that invokes com.facebook.impagepipeline.request ImageRequest

Solution

Implement a null check here

Additional Information

• Fresco version: 2.3.0
• Lastest commit: bd50311
• Platform version: React Native 0.63

[stale]

Hey there, it looks like there has been no activity on this issue recently. Has the issue been fixed, or does it still require the community's attention? This issue may be closed if no further activity occurs. You may also label this issue as "bug" or "enhancement" and I will leave it open. Thank you for your contributions.

[oprisnik]

Thanks for the bug report. Pull requests are welcome!

[gurshafriri]

👋 @oprisnik were you able to reproduce and verify the issue?
if so, were there any updates on this?
In case it's valid, we (at snyk) would like to add it to our vulnerability db. We would be happy to wait for a fix release if that's in plan.

[oprisnik]

Hey! Looking at this, you mention that the issue is in line 361 of ImageRequest, which seems to be an empty line https://github.com/facebook/fresco/blob/master/imagepipeline/src/main/java/com/facebook/imagepipeline/request/ImageRequest.java#L361

I'm also not sure what the exact NPE is, do you have a stack trace?

I'm not familiar with the getSize(...) method, what does that do. This seems more like a RN bug I believe.

[knadt]

Hi @oprisnik,

YES! you are correct, sorry. I have updated the description and changed the line number to 381.
When running the code on snack.expo.io this is the error I get.

.

Documentation on GetSize. The RN functionality for Image.getSize just calls into the Fresco library. The relevant code in React Native is here. Hence I thought the issue could be fixed here in Fresco deeper down. Please let me know your thoughts.

[knadt]

Hey @oprisnik.

any feedback? is this a RN issue and should I post this issue there? or can it be fixed here?

[knadt]

Hi @oprisnik, any feedback?

[oprisnik]

Hey! Sorry, I did not have the time to look into this. However both line 361 and 381 that you have linked to are empty lines and there's no mention of lastIndexOf and you didn't post the full stack trace yet. Can you please do that?

[knadt]

Hi @oprisnik
It was referencing MASTER and the file was updated. Please find the link to the ImageRequest.java.

The below (file:) crashes with the stack trace:
Uncaught Error: Attempt to invoke virtual method 'int java.lang.String.lastIndexOf(int)' on a null object reference

const demo = Image.getSize('file:',_=>{});

however file:// does not cause a crash
const demo = Image.getSize('file://',_=>{});

Like a said I am not a React developer.

[oprisnik]

Do you have more stack trace lines?
The file you linked at is using UriUtil internally and I cannot see any String operations there. So I guess this is either coming from React Native or Android (uri.getScheme() - which is an Android class)

[stale]

Hey there, it looks like there has been no activity on this issue recently. Has the issue been fixed, or does it still require the community's attention? This issue may be closed if no further activity occurs. You may also label this issue as "bug" or "enhancement" and I will leave it open. Thank you for your contributions.