File Upload Form Tag and Function

[Matthew Woodward]

I wanted to let everyone know that the <form:file> custom tag I mentioned a while ago is (or will be tonight) in the nightly builds. You can read more in the docs:
https://greatbiztoolsllc.trac.cvsdude.com/mach-ii/wiki/MachII1.8Speci...

It's pretty straight-forward but I'd love it if some of our users could put it through its paces.

Note that we wound up *not* appending _-_file_-_ on the end of the form field when the HTML is output. That was originally proposed because we were considering having the file be automatically uploaded, but we quickly decided that was a bad idea from a validation and security standpoint so it's no longer needed.

There's still time to comment on syntax, etc. so if you don't like what you see, please speak up!

Thanks.

--
Matthew Woodward
ma...@mattwoodward.com
http://mpwoodward.posterous.com
identi.ca/Twitter: @mpwoodward

Please do not send me proprietary file formats such as Word, PowerPoint, etc. as attachments.
http://www.gnu.org/philosophy/no-word-attachments.html

[Brian Klaas]

Hi Matt -

It's not clear (though I guess I could dissect the latest nightly in
the repository) if the fileUpload() function will first upload the
file to a temp, non-Web accessible directory and then move it to the
specified destination directory, or if it just uploads the file to the
destination directory.

This gets to the <cffile> upload issue that plagued a lot of Web sites
a couple of months ago, including Ben Forta's and House of Fusion,
where because they were allowing uploads but not first uploading them
to a non-Web accessible directory, hackers were able to request those
files in the milliseconds between upload and deletion the file once it
was determined to be malicious. I don't know if it's Mach-II's place
to worry about security and this particular issue, but it seems to me
that if the fileUpload() isn't taking this in to account, it's use
will be pretty curtailed by most people who use Mach-II. I know that I
wouldn't use it for just this security vulnerability.

brian

On Nov 16, 7:10 pm, Matthew Woodward <m...@mattwoodward.com> wrote:
> I wanted to let everyone know that the <form:file> custom tag I mentioned a
> while ago is (or will be tonight) in the nightly builds. You can read more
> in the docs:https://greatbiztoolsllc.trac.cvsdude.com/mach-ii/wiki/MachII1.8Speci...
>
> It's pretty straight-forward but I'd love it if some of our users could put
> it through its paces.
>
> Note that we wound up *not* appending _-_file_-_ on the end of the form
> field when the HTML is output. That was originally proposed because we were
> considering having the file be automatically uploaded, but we quickly
> decided that was a bad idea from a validation and security standpoint so
> it's no longer needed.
>
> There's still time to comment on syntax, etc. so if you don't like what you
> see, please speak up!
>
> Thanks.
>
> --
> Matthew Woodward

> m...@mattwoodward.comhttp://mpwoodward.posterous.com

[Matthew Woodward]

On Tue, Nov 17, 2009 at 9:01 AM, Brian Klaas <brian...@gmail.com> wrote:

Hi Matt -

It's not clear (though I guess I could dissect the latest nightly in
the repository) if the fileUpload() function will first upload the
file to a temp, non-Web accessible directory and then move it to the
specified destination directory, or if it just uploads the file to the
destination directory.

So it's clear, this new functionality is doing *nothing* in terms of low-level stuff that would in any way replace or bypass the normal way file uploads work in the CFML engines. It is literally a convenience wrapper for CFFILE. That's it. So any caveats and concerns with CFFILE and file uploads in general will apply here. We're not rebuilding that functionality, we're just making it simpler to use in the context of Mach-II.
 

I don't know if it's Mach-II's place
to worry about security and this particular issue, but it seems to me
that if the fileUpload() isn't taking this in to account, it's use
will be pretty curtailed by most people who use Mach-II. I know that I
wouldn't use it for just this security vulnerability.

I think you'll have to elaborate on this, because unless I'm missing something we're not introducing any new concerns here. Without using the tag and function in Mach-II, when you post a form containing a file upload a temp file is created automatically by the CFML engine. You then use CFFILE to put that uploaded file where you want it. When using fileUpload(), that function literally wraps CFFILE. That's all it does.

If you want to look at the code, it's in MachII/framework/EventContext.cfc, starting at line 711.

If I'm misunderstanding the issues you're bringing up please enlighten me, because we certainly don't want to introduce any new security concerns. But I do want to make it very clear that we are not changing the way file uploads work at a low level in any way whatsoever.

--
Matthew Woodward
ma...@mattwoodward.com

[Brian Klaas]

Hi Matt -

Thanks for clarifying things. As the tag is simply a wrapper for
CFFILE and returns the same set of data that CFFILE returns, it's then
up to the developer to implement good security practices when dealing
with file uploads even though the fileUpload() function takes care of
the actual file upload for them.

For reference, the vulnerability to which I referred is detailed here:

http://www.coldfusionmuse.com/index.cfm/2009/9/18/script.insertion.attack.vector

Maybe something could go in to the final docs which reminds developers
that it's up to them to implement good security practices and upload
to a non-Web-accessible directory, check to make sure that it's a
valid file using the information returned form the fileUpload()
function, then move the file somewhere else for more permanent storage
if it's an image or something else that needs to be in a Web-
accessible directory.

Or I could add that to the wiki. =)

brian


On Nov 17, 12:54 pm, Matthew Woodward <m...@mattwoodward.com> wrote:

> m...@mattwoodward.comhttp://mpwoodward.posterous.com

[Matthew Woodward]

On Tue, Nov 17, 2009 at 10:30 AM, Brian Klaas <brian...@gmail.com> wrote:

Or I could add that to the wiki. =)

 

That would be great if you could do that. I completely agree that even though we're not doing anything new here, having a reminder about exploits such as the one you point out would be a nice addition.

If you don't have time to add that let me know and I'll get on it. Thanks for the feedback and discussion on this!

Matt

--
Matthew Woodward
ma...@mattwoodward.com

[Peter J. Farrell]

This begs the question should we help (but not dictate) some security guidelines by having some defaults? For example, the "destination" argument could default to the directory returned by getTempDirectory().  There by we make destination an optional argument which defaults to the temp directory (which won't be web accessible).  The default would apply if the destination argument is left at a zero length string as well..

Also, in regards to the the "accept" argument to the method call (not the cffile tag) only accepts a comma-delimited list of MIME types.  I'm wondering if two improvements can be made here.

a) Allow arrays for this argument and auto-convert arrays to list using arguments.accept = ArrayToList(arguments.accept)

b) Full MIME types are always mediaType/subType format.  I don't know about you all, but I cannot for the life of me remember the all the MIME types I use.  We could easily use a UDF to convert file extension (.jpg, .gif, .doc, etc.) to the correct MIME type.

Another thing here, do we need to wrap the <cffile> with a try/catch so we can "rethrow" an exception more easily handled by a developer's calling code?  If I remember correctly, the exception "type" of cffile isn't something standard like like "application" or "file" -- it's some "com.allaire.". I haven't tested recently so I could stand to be corrected.

Yet another area of "oh, I need to go look that up" is when on *nix systems that the "mode" attribute of cffile only
accepts octals (777, 644, etc.).  If you're like me, I can *never* remember because I'm more familiar with using references when using cfmod.  I propose that we convert any non-numeric data for the "mode" argument of this helper function to the correct octal for people.

uploadFile("picture", "", "", ".jpg", "a=r o=wx")

The above code looks for a file in "picture", the accept type of ".jpg" is converted to image/jpeg (see I probably got this MIME wrong) and the mode is converted to 744 (I had to look this up for rwe for owner and r for all).

Maybe I'm over engineering this, but these are all the little sticking point I've had with cffile upload.

Penny for all y'all's thoughts?

.Peter


Brian Klaas said the following on 11/17/2009 12:30 PM:

[Brian Klaas]

I think that the MIME-type autoconversion is a great idea. You're not
going to be able to cover every possible MIME-type, but if you can get
a decent list compiled and make anything not on the list default to
something like "application/octet-stream," you'd cover 95% of all use
cases.

I'm not sure about the octal conversion, though, because, in my
experience, people who tend to run *nix servers to also remember the
octal values for file permissions.

I'm also not sure that wrapping a cffile exception will bring much
value. Now that I understand that the intended purpose of the
uploadFile() function is just to wrap <cffile> more nicely, I think
that things like catching errors and (as mentioned above) basic
security practices are really the domain of the developer implementing
a specific solution.

Just my $.02.

brian

> >http://www.coldfusionmuse.com/index.cfm/2009/9/18/script.insertion.at...