Protect against malicious blob names in Local File System Storage

2020IP / TwentyTwenty.Storage

A cross-cloud storage abstraction

Other

100 stars 26 forks source link

Protect against malicious blob names in Local File System Storage #26

Closed Kukks closed 5 years ago

Kukks commented 5 years ago

Email me at evilkukka@gmail.com for more info

jarroda commented 5 years ago

This looks great. Thanks!

Kukks commented 5 years ago

No problem, will you do a release soon since this is a bit of vulnerability?

ericgreenmix commented 5 years ago

Just pushed as 2.11.1. Should be indexed by Nuget shortly

Kukks commented 5 years ago

Awesome, thanks!

NicolasDorier commented 5 years ago

IMHO, this fix is fragile code. The proper way to fix it is to use Path.Combine and then verify on the full path that the path start with dir AFTER the concat with the blobName.

There is no need of regex, the regex part is complicated to read, maybe not right (are you sure this is the only way on all plateforms to prevent this type of attack?) and time consuming.

Also limit the blob name to ASCII alphabet...

NicolasDorier commented 5 years ago

Sorry, talked with Kukks and the check is correctly done. Though the regex is useless and wrong. (matching the first char which is ASCII)

ericgreenmix commented 5 years ago

@NicolasDorier Feel free to submit a cleanup or better solution. Thanks

ericgreenmix commented 5 years ago

@NicolasDorier and @Kukks a new version 2.11.2 is on its way to Nuget that removes the Regex check, because you are right, it is not needed.