Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

GetCombinedStreamAsync > GetRequiredFileInfo exception should be handled gracefully to prevent DDoS #199

Open
LennardF1989 opened this issue Dec 18, 2023 · 2 comments · May be fixed by #200
Labels

Comments

@LennardF1989
Copy link

LennardF1989 commented Dec 18, 2023

Upon inspecting logs of an Umbraco website, we saw an unhandled exception similar to below occur quite often:

System.IO.FileNotFoundException: No such file exists 202303303/019e3e65.js (mapped from 202303303/019e3e65.js)

File name: '202303303/019e3e65.js'
   at Smidge.InMemory.MemoryCacheFileSystem.GetRequiredFileInfo(String filePath)
   at System.Linq.Enumerable.SelectListPartitionIterator`2.MoveNext()
   at System.Linq.Enumerable.WhereSelectEnumerableIterator`2.ToList()
   at Smidge.Controllers.SmidgeController.GetCombinedStreamAsync(IEnumerable`1 files, BundleContext bundleContext)
   at Smidge.Controllers.SmidgeController.Composite(CompositeFileModel file)

After investigation, this is relatively easy to trigger by taking the URL and purposely including a wrong file-id, for example:
https://domain.tld/sc/598e8198.1234abcd.js.v202303303 where 1234abcd is a non-existing file.

This code currently throws an exception, but is never properly handled, causing a 500:

throw new FileNotFoundException($"No such file exists {fileInfo.PhysicalPath ?? fileInfo.Name} (mapped from {filePath})", fileInfo.PhysicalPath ?? fileInfo.Name);

(This is also the case for other FileSystem types).

This particular line seems to be the reason for the actual 500 happening:

var files = file.ParsedPath.Names.Select(filePath =>
_fileSystem.CacheFileSystem.GetRequiredFileInfo(
$"{file.ParsedPath.CacheBusterValue}/{filePath + file.Extension}"));

GetCombinedStreamAsync tries to do an if-Exists check, but you can't check .Exists on something that is not there. A possible solution instead of throwing an Exception is just logging that it occured, but returning a default FileInfo.

While this particular issue may seem like a non-issue and only occurs when caching is out of sync with the actual smidge files, being able to consistently trigger a 500 can be considered a DDoS vulnerability. As a lot of exceptions in a row will make IIS go into panic mode and restart the website.

@Shazwazza
Copy link
Owner

Hi, thanks for the report. Any chance can make a PR to resolve this? I'd be happy to log this instead of throw.

@Shazwazza Shazwazza added the bug label Dec 18, 2023
@LennardF1989
Copy link
Author

Sure, I can take a look :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants