Refinement & Fix bugs

[TheXerr0r]

During my research on the laravelmedia-library package source I figured out that two helper/Support functions on this package making problems in real-life cases, The first one was getHumanReadable() support function which if it receives a negative value or 0, the value will still be negative or it will return the (0) value as (0 KB) which is not relevant because the value of (0) means that there's no data inside a file so it should display (0 B) as it's more relevant and describe that the file has no data on it like it may be an empty .txt file, I know this may not happen in real life development as the function is being used internally, but the calculation of the function is wrong and may cause problems in future, So the function has been refactored and the mentioned bugs have been fixed you can test it in php artisan tinker command from both my fixes & the previous code to assure that it's a good commit.

Second, the createMultipleFromRequest() function from the FileAdderFactory::class seems to be vulnerable with an LFI (Local File Inclusion) vulnerability, to prove it you can execute this part of the code on the php artisan tinker console:

file_get_contents(str_replace(['[', ']', '"', "'"], ['.', '', '', ''], "../../../../../../../etc/hosts"))

I know that this command is just an execution of a malicious payload and not related to the function somehow, but I've provide that to prove that the filtration that has been done on the file is not working well and it's vulnerable to LFI, so thinking of the vulnerable function pushes me to fork the repo to make the package safer as it's the biggest one that is mostly being used on laravel applications.

And it's worth mentioning to say that if you chain the above payload mechanism you can actually bypass the filtration and force the function to upload an internal server file like /etc[/hosts, /passwd, /shadow, ...etc] to the storage bucket of the Laravel application that is using this file, it can be done by manipulating the HTTP POST request that accepting the multipart/formdata header, it's doable as the user has access over the createMultipleFromRequest() method using some other methods on the Traits.

In the end, thanks for your time in reading my explanation, I'll be happy to be a collaborator in this repo if it finds me creative and reasonable to be.

Cheers,
Ehsan Faramarz - TheXerr0r

[freekmurze]

Could you add tests to make sure the new behaviour works as intended?

[TheXerr0r]

Done sir, sorry I was too busy so I forgot to check the tests, now the code is working with your current tests. and this is the result that I've got from the php artisan tinker console:

> trim(basename('../../../../etc/passwd.jpeg.../'), './')
= "passwd.jpeg"


> trim(basename('../../../../etc/pass-wd.jpeg.../'), './')
= "pass-wd.jpeg"

so it means that it's working with all the possible/valid file names, as far as coming to getHumanReadableSize() support function it's working fine with all test cases. Please check both and let me know if there's a mistake on my changes.

Regards,
Ehsan Faramarz - TheXerr0r

[freekmurze]

Thanks!