Task terminated with unhandled exception: Invalid path entry

[marcioapm]

Task terminated with unhandled exception: Invalid path entry: H:\Own files\Eigene Dokumente\Travel\160704_Ihla.docx

We get these sometimes. It seems like some browser or someone malicious is passing some oddly formatted string as the file name, which vibe fails to parse. This is an assert in the PathEntry ctor.
It happens while parsing the request, before any of my code runs.

Unfortunately, I have no idea what the full path passed is nor do I have a callstack, but I assume it's being called from splitPath()

I think sanitation should happen before hand, and errors should be gracefully dealt with in this case. I could then handle bad paths on my application code, but as it is, I have no chance of doing so.

[marcioapm]

More info: core.exception.AssertError@../../../../root/.dub/packages/vibe-d-0.7.30/vibe-d/source/vibe/inet/path.d(426): Invalid path entry: U:\PERSO\25-3-17_Ch_Blr.docx

@s-ludwig any chance of a quick fix? anything will work, but killing the event loop with an assert makes this an easy DoS channel

[s-ludwig]

I'll have a look at this. It's probably okay (necessary anyway) to relax the rules a bit. The new Path implementation in vibe-core still has this issue, but it can be fixed there, because the path has a kind associated with it (windows, posix or inet).

However, in this case the right thing to do would be to parse this as a Windows path, whereas it will currently be parsed as a system native path. So to maybe find a higher level solution, do you know where the path originates from? A form upload? vibe-core defines and uses a new PathEntry.validateFilename function that I could backport to 0.7.31.

In addition to converting the assertion to an exception, the form upload code could then also attempt to sanitize the file name, of course.

[glathoud]

Hello, hopefully this helps: I just observed this on one of my vibe.d-based servers:

Task terminated with unhandled exception: Invalid path entry: %{(#test='multipart/form-data').(#[email protected]@DEFAULT_MEMBER_ACCESS).(#_memberAccess?(#_memberAccess=#dm):((#container=#context['com.opensymphony.xwork2.ActionContext.container']).(#ognlUtil=#container.getInstance(@com.opensymphony.xwork2.ognl.OgnlUtil@class)).(#ognlUtil.getExcludedPackageNames().clear()).(#ognlUtil.getExcludedClasses().clear()).(#context.setMemberAccess(#dm)))).(#[email protected]@getRequest()).(#[email protected]@getResponse()).(#res.setContentType('text/html;charset=UTF-8')).(#res.getWriter().print('security_')).(#res.getWriter().print('check')).(#res.getWriter().flush()).(#res.getWriter().close())}

This seems to be related to an Apache Struts2 exploit: rapid7/metasploit-framework#8064

[Geod24]

This commit turns the assert into an Exception which can then be handled by user code. Additionally, tasks that aren't nothrow are now deprecated, so the "Task terminated with unhandled exception" error is going away, forcing the user to explicitly handle the exceptions.

Feel free to comment / re-open if I missed anything.