External Control of File Name or Path (Veracode CWE ID 73)

[leviyakir]

We have used Veracode to scan Akavache library and it has come up with this issue:

Description

Allowing user input to control paths used in filesystem operations may enable an attacker to access or modify otherwise
protected system resources that would normally be inaccessible to end users. In some cases, the user-provided input may be
passed directly to the filesystem operation, or it may be concatenated to one or more fixed strings to construct a fully-qualified
path.
When an application improperly cleanses special character sequences in user-supplied filenames, a path traversal (or
directory traversal) vulnerability may occur. For example, an attacker could specify a filename such as "../../etc/passwd",
which resolves to a file outside of the intended directory that the attacker would not normally be authorized to view.

Recommendations

Assume all user-supplied input is malicious. Validate all user-supplied input to ensure that it conforms to the expected format,
using centralized data validation routines when possible. When using black lists, be sure that the sanitizing routine performs a
sufficient number of iterations to remove all instances of disallowed characters and ensure that the end result is not
dangerous.

The specific reference is to the methods CreateStreamFromUri and CreateStreamFromPath that are reachable externally:
https://github.com/castleproject/Core/blob/master/src/Castle.Core/Core/Resource/UncResource.cs
https://github.com/castleproject/Core/blob/master/src/Castle.Core/Core/Resource/FileResource.cs

Description

This call contains a path manipulation flaw. The argument to the function is a filename constructed using untrusted
input. If an attacker is allowed to specify all or part of the filename, it may be possible to gain unauthorized access to
files on the server, including those outside the webroot, that would be normally be inaccessible to end users. The level
of exposure depends on the effectiveness of input validation routines, if any.

Recommendations

Validate all untrusted input to ensure that it conforms to the expected format, using centralized data validation routines
when possible. When using black lists, be sure that the sanitizing routine performs a sufficient number of iterations to
remove all instances of disallowed characters.

[stakx]

I'm not really sure I understand what we're being asked to do here.

The argument to the function is a filename constructed using untrusted input. If an attacker is allowed to specify all or part of the filename, it may be possible to gain unauthorized access to files on the server, including those outside the webroot, that would be normally be inaccessible to end users.

Castle.Core is a fairly generic library and as such it can execute in a variety of environments; web servers are only one of them. That is, we simply don't know whether we're running in a trusted or untrusted environment. Therefore, perhaps the path validation needs to happen outside of Castle.Core, in the downstream application code?

(Besides, that part of Castle.Core hasn't seen any active development AFAIK. Seems a little stale to me, I for one would rather invest my dev time in DynamicProxy.)

[jonorossi]

@stakx I agree.

I think IResource and its implementations should move into Windsor, it is no longer used by MonoRail, ActiveRecord, etc like it used to be, only Windsor survived. Obviously needs a major version upgrade, so happy to just ignore it for now like other areas like logging and just focus on DP for now until next major version.