[Mono-dev] Bug in mono 3.0.1 MVC3 File/FileResult

SirNoSkill quandary82 at hailmail.net
Thu Feb 7 07:09:25 UTC 2013


Granted, letting it run as root isn't a good idea, to say the least.

But I had too few time to deal with permission issues.

So in order to deal with the problem, I added some extension methods
(see below) and will just use these in the future.

My TransmitFile.ashx handler should be back in safely with that,
calling SafeTransmitFile instead.



PS:

I figured the problem might be a missing nginx configuration parameter,
in which case it would be a documentation bug.

So I googled a bit, and tried a bit.



Based on the quora links, I added chunked_transfer encoding,
proxy_http_version and fastcgi_keep_conn to the virtual name based
hosting configuration of nginx.



location / {

root /home/danillo/www/HomePage;

#index index.html index.htm default.aspx Default.aspx;

#fastcgi_index Default.aspx;

fastcgi_pass 127.0.0.1:9000;

include /etc/nginx/fastcgi_params;

chunked_transfer_encoding on;

proxy_http_version 1.1;

#fastcgi_keep_conn on;

}



to the nginx virtual name based hosting config for that domain.



That doesn't make a difference (i restarted both nginx and fastcgi
after the config change).

Also, switching proxy_http_version to 1.0 and
chuncked_transfer_encoding to off doesn't make a difference.



But it should.

As the fastcgi-interface is set to 1.0, it shouldn't return a
chunked_transfer_encoding at all, since this is part of http 1.1.

Unless I misplaced this config setting by putting it into location.


using System;

using System.Collections.Generic;

using System.Linq;

using System.Web;


namespace System

{


    public static class SafetyExtensions

    {

        private static string m_strTempPath =
System.IO.Path.GetTempPath();


        public static void SanityCheck(string
strUntrustedFileNameAndPath)

        {

            if (string.IsNullOrWhiteSpace(strUntrustedFileNameAndPath))

                throw new
ArgumentNullException("strUntrustedFileNameAndPath");

            string strAbsoluteFileNameAndPath =
System.IO.Path.GetFullPath(strUntrustedFileNameAndPath);

            string strFileName =
System.IO.Path.GetFileName(strAbsoluteFileNameAndPath);

            if (string.IsNullOrWhiteSpace(strAbsoluteFileNameAndPath))

                throw new NullReferenceException("strFileNameAndPath");

            if (string.IsNullOrWhiteSpace(strFileName))

                throw new NullReferenceException("strFileName");

            if
(!strAbsoluteFileNameAndPath.StartsWith(AppDomain.CurrentDomain.BaseDir
ectory, StringComparison.OrdinalIgnoreCase))

            {

                if
(!strAbsoluteFileNameAndPath.StartsWith(m_strTempPath,
StringComparison.OrdinalIgnoreCase))

                    throw new UnauthorizedAccessException("Error: Path
\"" + strAbsoluteFileNameAndPath + "\" is below the application root
directory.");

            }


            if
(strAbsoluteFileNameAndPath.StartsWith(System.IO.Path.Combine(AppDomain
.CurrentDomain.BaseDirectory, "App_Data"),
StringComparison.OrdinalIgnoreCase))

                throw new UnauthorizedAccessException("Error: App_Data
is a confidential directory.");

            if (StringComparer.OrdinalIgnoreCase.Equals(strFileName,
"web.config"))

                throw new UnauthorizedAccessException("Error: Access
for any file called \"web.config\" is denied.");

            if (StringComparer.OrdinalIgnoreCase.Equals(strFileName,
"connections.config"))

                throw new UnauthorizedAccessException("Error: Access
for any file called \"connections.config\" is denied.");

            if (StringComparer.OrdinalIgnoreCase.Equals(strFileName,
"elmah.config"))

                throw new UnauthorizedAccessException("Error: Access
for any file called \"elmah.config\" is denied.");

        } // End Sub SanityCheck


        public static string SafeMapPath(this HttpServerUtility srv,
string path)

        {

            string strRet = srv.MapPath(path);

            SanityCheck(strRet);

            return strRet;

        }


        public static void SafeTransmitFile(this HttpResponse resp,
string filename)

        {

            SanityCheck(filename);

            resp.TransmitFile(filename);

        }


        public static void SafeWriteFile(this HttpResponse resp, string
filename)

        {

            SanityCheck(filename);

            resp.WriteFile(filename);

        }

    }

}





On Sun, Feb 3, 2013, at 01:57 PM, Daniel Lo Nigro wrote:

  RFI can't work in MVC

Yeah, routing rules should block it, I forgot to mention that. I don't
think [1]ASP.NET MVC allows "\" in its route parameters.

But if you have the default route (/ControllerName/ActionName) enabled,
your app could still be vulnerable. A user could pass the parameter as
a GET or POST parameter (ie. go to
/Gallery/FullImage?id=../../../../../../../etc/passwd) and the default
model binder will accept this parameter. It's usually safer to always
do validation of your parameters instead of relying on the routing
engine to do it :)

  What I mean is file path validation in
  Response.TransmitFile
  Response.WriteFile
  Server.MapPath
  System.IO.Path.GetFiles
  etc.
  To check whether the requested file is not below the root directory
  of the web application

But in some cases you might want to read files below the root directory
(eg. some apps use c:\Windows\Temp or /tmp)

  WriteFile.ashx?myfile=../../../../../../../root/.ssh/id_rsa would be
  really really bad.

This should never work as id_rsa should have its mode set to 0700 and
Mono shouldn't be running as root. The user Mono runs as should be
relatively locked down. I use www-data (the default web server /
PHP-FPM user in Debian) for mine.



On Mon, Feb 4, 2013 at 12:03 AM, quandary <[2]quandary82 at hailmail.net>
wrote:

Remote file inclusion fixed, ashx handlers removed, FullImage removed,
website back up.

Filed but 10'001
[3]https://bugzilla.xamarin.com/show_bug.cgi?id=10001



No, I don't mean parameter validation, and RFI can't work in MVC when
you request from a browser on Windows, because
parameters are separated by / and windows translates backslash to
forwardslash.
(at least not until one uses a catchall parameter), I checked.

If you'd use a browser on Linux, I don't know if it would change
backslashes into slashes,
which would be a potentially dangerous thing for a windows server.
But I have a Linux server, so who cares about that.

It can only work for parameters passed via QueryString/HttpPost, such
as in the two ashx handlers I added.
(or if a confidential file is in the same directory, but that would be
really stupid).


What I mean is file path validation in
Response.TransmitFile
Response.WriteFile
Server.MapPath
System.IO.Path.GetFiles
etc.
To check whether the requested file is not below the root directory of
the web application
(so that it would throw an AccessDeniedException on TransmitFile).

Or in other words,
if( !strFileName.StartsWith(AppDomain.CurrentDomain.BaseDirectory,
StringComparer.OrdinalIgnoreCase)
     throw new AccessDeniedException("no access to files below
application root directory");

of course, the above is not sufficient, because relative paths in
absolute paths are possible and supported by .NET/Windows/Linux.

Because if that path validation isn't done, one can request (for
example in my previous handler)
wget
[4]http://www.daniel-steiger.ch/WriteFile.ashx?myfile=../../../../../..
/../etc/passwd
which makes RFI interesting in the first place.
I checked an it worked, I got /etc/passwd back...
Now /etc/passwd wouldn't be that bad,
since it only contains MD5 hashes (though MD5 is rainbow-table
vulnerable) and because I configured ssh to not allow password logins,
but WriteFile.ashx?myfile=../../../../../../../root/.ssh/id_rsa would
be really really bad.

I think I remember stumbling over such an exception somehow in IIS
(perhaps SecurityException and not AccessDenied),
but not on the [5]ASP.NET development server.

On 02/03/2013 12:41 PM, Daniel Lo Nigro wrote:

Better I mention it than risking someone more malicious noticing it,
since the link was already in a public mailing list. :)

  Isn't this a mono-bug, too ?

As far as I'm aware, the .NET Framework only validates for HTML tags in
parameters. It doesn't validate file paths since it doesn't even know
the parameter will be used for a file path (things like "..\" could be
valid GET parameters for your page). I don't think there's any built in
mechanism to prevent directory traversal.

.NET request
validation: [6]http://msdn.microsoft.com/en-us/library/hh882339.aspx



On Sun, Feb 3, 2013 at 10:34 PM, quandary <[7]quandary82 at hailmail.net>
wrote:

Oh wonderful, it's called remote file inclusion.
I suspected that much, but I didn't bother to address it,
because I didn't publish the sources and internal config files - up
until today.

So with you having mentioned it for all script kiddies to see - site
taken down until validation is added.
Before that, I quickly checked - one can access files below the root
directory of the web application.

Isn't this a mono-bug, too ?
Because I think I remember me having done this once on a test or
production server, and it gave a wonderful YSOD on IIS.

On 02/03/2013 11:45 AM, Daniel Lo Nigro wrote:

That does look like a bug with how Mono handles TransmitFile - I
suggest reporting it as a bug in Xamarin Bugzilla (report it under the
System.Web component).

Also FYI it's probably best if you pull down those pages for now;
you're not validating the "myfile" parameter so it's open to a
[8]Remote File Inclusion vulnerability.



On Sun, Feb 3, 2013 at 9:38 PM, quandary <[9]quandary82 at hailmail.net>
wrote:

Yep, indeed that sounds like that.
And I just tested.
Added WriteFile.ashx and Transmit.ashx

and testet with
[10]http://www.daniel-steiger.ch/WriteFile.ashx
[11]http://www.daniel-steiger.ch/Transmit.ashx
and
[12]http://www.daniel-steiger.ch/WriteFile.ashx?myfile=avatar100.png
[13]http://www.daniel-steiger.ch/Transmit.ashx?myfile=avatar100.png


It seems the bug is in Response.TransmitFile for files of any size
(also for avatar100.png, which is only 4.3 kb)

so to summarize, there is a rather bad-natured bug in
Class: System.Web.HttpResponse
Method: TransmitFile(string filename)


This is the transmit-handler code:

using System;
using System.Collections.Generic;
using System.Linq;
using System.Web;

namespace Homepage
{
    /// <summary>
    /// Zusammenfassungsbeschreibung für Transmit
    /// </summary>
    public class Transmit : IHttpHandler
    {

        public void ProcessRequest(HttpContext context)
        {
            string strFile = context.Request.Params["myfile"];

            if (string.IsNullOrEmpty(strFile))
                strFile = "001.jpg";

            string strNetPath =
string.Format("~/Content/images/gallery/{0}", strFile);
            string strFileNameAndPath =
context.Server.MapPath(strNetPath);

            context.Response.Clear();
            context.Response.ContentType = "image/jpeg";
            context.Response.TransmitFile(strFileNameAndPath);
        }

        public bool IsReusable
        {
            get
            {
                return false;
            }
        }
    }

}



Regards

Stefan

On 02/03/2013 06:14 AM, Daniel Lo Nigro wrote:

That sounds like chunked encoding, Wikipedia says
([14]http://en.wikipedia.org/wiki/Chunked_transfer_encoding):
Each chunk starts with the number of octets of the data it embeds
expressed in hexadecimal followed by optional parameters (chunk
extension) and a terminating CRLF sequence, followed by the chunk data.
The chunk is terminated by CRLF. If chunk extensions are provided, the
chunk size is terminated by a semicolon followed with the extension
name and an optional equal sign and value.

Which is exactly what you're saying. I wonder if something is not being
done correctly with files as large as the ones you're using. Since you
said it works for thumbnails, I assume it's working for smaller files.

Try Response.WriteFile or Response.TransmitFile in a standard
[15]ASP.NET handler (.ashx) and see if they also don't work.

  All traffic to that URL [[16]www.daniel-steiger.ch] (except for the
  folders /doc and /images), but including images in /Content, is
  directly forwarded to fastcgi by nginx, as per fastcgi config file
  for domain.

I'd still suggest letting Nginx serve your static files. Just because
the site is low-traffic doesn't mean that little performance tweaks
aren't good :). I do something like this:
location / {
# Pass requests for unknown files to Mono
try_files $uri @mono;
}

location @mono {
# Put all your Mono config here
}
My full site config is
at [17]https://github.com/Daniel15/Website/blob/master/nginx.conf



On Sun, Feb 3, 2013 at 4:00 PM, SirNoSkill
<[18]quandary82 at hailmail.net> wrote:

I have more details on the bug.
The extra bytes that are at the beginning
31 39 36 62 36 38 0D 0A

which reads 196b68/r/n in ASCII
196b68 is the filesize of the original image in hex...

All details + hexdump links added here:
[19]http://stackoverflow.com/questions/14662795/why-do-i-have-unwanted-
extra-bytes-at-the-beginning-of-image



All traffic to that URL [[20]www.daniel-steiger.ch] (except for the
folders /doc and /images), but including images in /Content, is
directly forwarded to fastcgi by nginx, as per fastcgi config file for
domain.


 server {
         listen   80;
         server_name [21]www.daniel-steiger.ch[22]daniel-steiger.ch;
         access_log   /var/log/nginx/daniel-steiger.ch.access.log;

         location / {
                 root /home/danillo/www/HomePage;
                 #index index.html index.htm default.aspx Default.aspx;
                 #fastcgi_index Default.aspx;
                 fastcgi_pass [23]127.0.0.1:9000;
                 include /etc/nginx/fastcgi_params;
         }


location /doc {
root /usr/share;
autoindex on;
allow 127.0.0.1;
deny all;
}

location /images {
root /usr/share;
autoindex off;
}

#error_page 404 /404.html;

# redirect server error pages to the static page /50x.html
#
error_page 500 501 503 504 /50x.html;
location = /50x.html {
root /home/danillo/www/HomePage;
}


error_page 502 /502.html;
location = /502.html {
root /home/danillo/www/HomePage;
}

}


It's sufficient to have the file served without FileResult.
Of course it's more efficient if nginx serves it directly, but this is
a very low traffic website, so performance is really not my problem ;)

And by the way, the problem is not finding a workaround.
I have already fixed it with a workaround about a week ago.
I really just want to know where the bug is, because if FileResult
malfunctions, there's probably more to it, and I don't want to walk
into a subtle not at the first sight spottable bug later, like a
botched binary upload/download file.





On Sat, Feb 2, 2013, at 06:51 AM, Daniel Lo Nigro wrote:

Hmm... Maybe try an X-Accel-Redirect header instead. This lets Nginx
serve the file instead of Mono having to serve it, which makes it more
efficient. See if that makes a difference, or if it has the same issue.

Why not just link directly to the file, instead of serving it through
your C# code?



On Sun, Feb 3, 2013 at 1:43 AM, quandary82
<[24]quandary82 at hailmail.net> wrote:

Corrected the mime, but seems to be a mono-bug (or fastcgi) anyway.



More here:

[25]http://stackoverflow.com/questions/14662795/why-do-i-have-unwanted-
extra-bytes-at-the-beginning-of-image







--

View this message in context:
[26]http://mono.1490590.n4.nabble.com/Bug-in-mono-3-0-1-MVC3-File-FileR
esult-tp4658382p4658422.html

Sent from the Mono - Dev mailing list archive at Nabble.com.

_______________________________________________
Mono-devel-list mailing list
[27]Mono-devel-list at lists.ximian.com
[28]http://lists.ximian.com/mailman/listinfo/mono-devel-list


--
SirNoSkill
[29]quandary82 at hailmail.net
--
[30]http://www.fastmail.fm - mmm... Fastmail...



--

SirNoSkill

quandary82 at hailmail.net

References

1. http://ASP.NET/
2. mailto:quandary82 at hailmail.net
3. https://bugzilla.xamarin.com/show_bug.cgi?id=10001
4. http://www.daniel-steiger.ch/WriteFile.ashx?myfile=../../../../../../../etc/passwd
5. http://ASP.NET/
6. http://msdn.microsoft.com/en-us/library/hh882339.aspx
7. mailto:quandary82 at hailmail.net
8. http://en.wikipedia.org/wiki/Remote_file_inclusion
9. mailto:quandary82 at hailmail.net
  10. http://www.daniel-steiger.ch/WriteFile.ashx
  11. http://www.daniel-steiger.ch/Transmit.ashx
  12. http://www.daniel-steiger.ch/WriteFile.ashx?myfile=avatar100.png
  13. http://www.daniel-steiger.ch/Transmit.ashx?myfile=avatar100.png
  14. http://en.wikipedia.org/wiki/Chunked_transfer_encoding
  15. http://ASP.NET/
  16. http://www.daniel-steiger.ch/
  17. https://github.com/Daniel15/Website/blob/master/nginx.conf
  18. mailto:quandary82 at hailmail.net
  19. http://stackoverflow.com/questions/14662795/why-do-i-have-unwanted-extra-bytes-at-the-beginning-of-image
  20. http://www.daniel-steiger.ch/
  21. http://www.daniel-steiger.ch/
  22. http://daniel-steiger.ch/
  23. http://127.0.0.1:9000/
  24. mailto:quandary82 at hailmail.net
  25. http://stackoverflow.com/questions/14662795/why-do-i-have-unwanted-extra-bytes-at-the-beginning-of-image
  26. http://mono.1490590.n4.nabble.com/Bug-in-mono-3-0-1-MVC3-File-FileResult-tp4658382p4658422.html
  27. mailto:Mono-devel-list at lists.ximian.com
  28. http://lists.ximian.com/mailman/listinfo/mono-devel-list
  29. mailto:quandary82 at hailmail.net
  30. http://www.fastmail.fm/

-- 
http://www.fastmail.fm - The professional email service

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.ximian.com/pipermail/mono-devel-list/attachments/20130206/b9300c9f/attachment-0001.html>


More information about the Mono-devel-list mailing list