2
\$\begingroup\$

Consider the following method that I have for checking write permissions on a directory path:

/// <summary>
/// Check existence and write permissions of supplied directory.
/// </summary>
/// <param name="directory">The directory to check.</param>
protected static void CheckPermissions(string directory)
{
    if (!Directory.Exists(directory))
    {
        throw new DirectoryNotFoundException(String.Format(JobItemsStrings.Job_DirectoryNotFound, directory));
    }

    // Check permissions exist to write to the directory.
    // Will throw a System.Security.SecurityException if the demand fails.
    FileIOPermission ioPermission = new FileIOPermission(FileIOPermissionAccess.Write, directory);
    ioPermission.Demand();
}

When running FxCop, this code throws up a "CA2103 - Review imperative security" warning, albeit with a certainty of 25%, with this info:

"Use of imperative demands can lead to unforeseen security problems. The values used to construct a permission should not change within the scope of the demand call. For some components the scope spans from the demand call to end of the method; for others it spans from the demand call until the component is finalized. If the values used to construct the permission are fields or properties, they can be changed within the scope of the demand call. This can lead to race conditions, mutable read-only arrays, and problems with boxed value types."

Bascially, is FxCop being over-cautious, or am I doing it wrong?

\$\endgroup\$
  • 1
    \$\begingroup\$ This is probably a question better suited for StackOverflow based on the FAQs guidelines for what is an applicable question. \$\endgroup\$ – Mark Loeser Jan 29 '11 at 17:28
  • \$\begingroup\$ @Mark: Hmm, well the FAQ says "If you are looking for specific feedback about… Code correctness, ..., Security issues in a code snippet, etc … then you are in the right place!" I figured my question was a pretty good fit for that. But, if it is felt that it better suited for SO, then fair enough. :) \$\endgroup\$ – Andy Jan 29 '11 at 17:45
3
\$\begingroup\$

This FxCop warning is basically asking you to make sure ("Review") that that non-constant you are passing (directory) to the security permission does not change while the permission is in effect. Basically, FxCop isn't sure if it is possible for the code (or some rogue module that a hacker has put in place) to do something like the following:

  1. Set directory to "c:\Temp\"
  2. .Demand()
  3. <untrusted>Set directory to "c:\Windows\System32\"</untrusted>
  4. Write something into a file contained in directory.

In this particular case, since directory is a non-ref parameter, it is not possible for another module outside your call-descendants to modify it. Thus, what you need to check for:

  • Anything in this method assigning a value to directory
  • Anything in this method that passes directory by reference (ref/out/unsafe pointers)

Disclaimer: I am not a code security expert and have no formal training as such. I may have missed entire classes of things to look for here. If you are dealing with code that has real-world security implications, I highly suggest you hire a consultant who does, rather than take what I wrote above as gospel.

\$\endgroup\$
  • \$\begingroup\$ Thanks, you have clarified what I thought. This does not have Earth-shattering implications - I just want to gracefully handle any circumstance where the permissions don't allow writing by popping a MessageBox. \$\endgroup\$ – Andy Jan 30 '11 at 13:47
0
\$\begingroup\$

When working with files, checking before an operation can be useful, but you still always need to handle relevant exceptions (e.g. FileNotFoundException, IOException).

The permissions (or existence) of a file/directory may change between the time you check and time the operation is invoked (new FileIOPermission(...) in this case). This situation is more common than it seems.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.