richardgirges / express-fileupload

Simple express file upload middleware that wraps around busboy
MIT License
1.52k stars 261 forks source link

Major security threats #312

Closed ClaudiuOctavianMuresan closed 2 years ago

ClaudiuOctavianMuresan commented 2 years ago

Please have a look at CVE-2022-27261 and CVE-2022-27140

richardgirges commented 2 years ago

Please provide additional details. Thanks.

ClaudiuMuresan commented 2 years ago

The links to mentioned CVEs should contain all details. Do you need more info? The details from the CVEs clearly provide the target of the vulnerabilities : express-fileupload

r3wt commented 2 years ago

@richardgirges Hi Richard, I reached out to the author of the attacks, Harun Oz, and asked him to contact you with the details. It's not clear from either video how the attack(s) work. Videos are from february, but just went into NIST on 4/12, which is kind of odd in my opinion.

vishal-bypt commented 2 years ago

Is there any ETA for CVE-2022-27261 and CVE-2022-27140 ?

richardgirges commented 2 years ago

@richardgirges Hi Richard, I reached out to the author of the attacks, Harun Oz, and asked him to contact you with the details. It's not clear from either video how the attack(s) work. Videos are from february, but just went into NIST on 4/12, which is kind of odd in my opinion.

Thanks @r3wt - exactly my thoughts

richardgirges commented 2 years ago

Because this is an unreviewed vulnerability and there are no relevant details explaining the attack, going to close this. Feel free to reopen when you have concrete and actionable information. Thanks!

harunoz commented 2 years ago

Dear all,

I sent a detailed explanation for each type of attack and potential countermeasures regarding our findings to a @richardgirges. I did not receive a response from him yet. Thanks.

jeffpm commented 2 years ago

Any update on this?

richardgirges commented 2 years ago

Any PRs would be appreciated

arxenix commented 2 years ago

@richardgirges has @harunoz contacted you with the vulnerability details? is this a legitimate finding?

richardgirges commented 2 years ago

I am looking into Harun’s findings now. Many of his proposed solutions is to add numerous NPM dependencies to express-fileupload, in addition to introducing major breaking changes. This seems unideal.

I will need to spend some time looking deeply into the findings to see if there is a way to address them, and to determine if the scope of these issues should fall under the responsibility of the user or express-fileupload.

duterte commented 2 years ago

Its a bluff, There is no security issues. express-fileupload does not write file without calling mv() method so how does that going to overwrite server files ?

It's up to developer on how and where to write file.

r3wt commented 2 years ago

Its a bluff, There is no security issues. express-fileupload does not write file without calling mv() method so how does that going to overwrite server files ?

It certainly seems that way. The video i saw featured a guy modifying files in notepad while uploading to localhost. i wanted to give the benefit of the doubt though and see if it was an actual vulnerability.

richardgirges commented 2 years ago

Its a bluff, There is no security issues. express-fileupload does not write file without calling mv() method so how does that going to overwrite server files ?

It's up to developer on how and where to write file.

I'm still reading through all of Harun's findings. Many of the issues in Harun's report appear to indicate intentional misusing of the API. Much of it overlaps with how the developer should be handling uploaded files and assumes express-fileupload should be handling the business logic of file renamings, etc - I'm not sure I agree with this.

There are some issues around safeFileNames that may be worthwhile to implement, but I need more time to confirm that these are legitimate security concerns that should fall under the purview of express-fileupload

richardgirges commented 2 years ago

Closing this ticket in favor of https://github.com/richardgirges/express-fileupload/issues/316