[Xerte-dev] Re: SECURITY PATCH for upload.php

Fri Mar 8 20:32:10 GMT 2013

Which installs don't use sessions? Edit.php bounces you id session id isn't set...

Regards

John Smith
Learning Technologist
School of Health and Life Sciences

Sent from Samsung Galaxy SII

"Pat @ Pgogy" <xerte at pgogywebstuff.com> wrote:

Argh stupid webmail not working

You can't rely on user_sessions as some installs don't use it - it is optional whether you use it and it is turned off by default. I wrote it in for toolkits 1.6 but then some server settings changed at Nottingham so I commented it out.

The session id won't make it safer as a malevolent person could still do stuff. Logically you move user-files so sit outside the web root, but that is a problem as flash is shite with relative paths.

The safest thing is to move userfiles outside web root

You could also add a .htaccess to prevent any php access and so on

Then you need a white list, given that models can only be added by admin then then allowing models to set the whitelist isn't a million miles from letting the models so it.

Once you've got the upload working then your next problem is that when upload fails no code exists to display an error, and it sounds like you need that to happen

The Wordpress plugin saves the XML using Ajax and uses a new upload URL to work

Pgogy Webstuff - http://www.pgogywebstuff.com
Makers of web things of a fair to middling quality

On 8 Mar 2013, at 15:13, "Smith, John" <J.J.Smith at gcu.ac.uk<mailto:J.J.Smith at gcu.ac.uk>> wrote:

I “think” we have 2 possible solutions:

I’m sure if we delegate jquery to do the upload then we can get at the progress and feed back to Flash, even if the Flash had to poll… there are jquery uploaders that do…

I think though we can also do it with session and  nonce though, but it is how we deal with multiple uploads without refresh – alternatively we just

·         Store the session id in the database, with a timestamp every time a php file loads, in config or something…

·         send the session id as a parameter upload.php?sess=<%php echo(session_id())%> into flash or as a flashvar

·         flash posts the session_id to upload and we interrogate the database to see if it’s valid (present and not expired – older than 20 mins

I’ll play around with it over the weekend…

Regards,

John Smith
Learning Technologist
School of Health & Life Sciences
Glasgow Caledonian University

From: xerte-dev-bounces at lists.nottingham.ac.uk<mailto:xerte-dev-bounces at lists.nottingham.ac.uk> [mailto:xerte-dev-bounces at lists.nottingham.ac.uk] On Behalf Of Julian Tenney
Sent: Friday, March 08, 2013 2:54 PM
To: For Xerte technical developers
Subject: [Xerte-dev] Re: SECURITY PATCH for upload.php

Maybe that’s not a bad idea, but is the case that you can’t get progress from the browser, which ideally we want for the progress bar (is that the case? I thought that was the downside of calling some js functions from the wizard to handle the call to upload.php?)

From: xerte-dev-bounces at lists.nottingham.ac.uk<mailto:xerte-dev-bounces at lists.nottingham.ac.uk> [mailto:xerte-dev-bounces at lists.nottingham.ac.uk] On Behalf Of Smith, John
Sent: 08 March 2013 14:21
To: For Xerte technical developers
Subject: [Xerte-dev] Re: SECURITY PATCH for upload.php

Hmm perhaps we can ajax via the browser…  now I see why nobody was wanting to touch this!!

Regards,

John Smith
Learning Technologist
School of Health & Life Sciences
Glasgow Caledonian University

From: xerte-dev-bounces at lists.nottingham.ac.uk<mailto:xerte-dev-bounces at lists.nottingham.ac.uk> [mailto:xerte-dev-bounces at lists.nottingham.ac.uk] On Behalf Of Julian Tenney
Sent: Friday, March 08, 2013 2:15 PM
To: For Xerte technical developers
Subject: [Xerte-dev] Re: SECURITY PATCH for upload.php

It’s because upload.php is being hit from flash, which isn’t passing the session info over to it in FF. Simon A and I tried a load of things to get it to work, but gave up in the end.

From: xerte-dev-bounces at lists.nottingham.ac.uk<mailto:xerte-dev-bounces at lists.nottingham.ac.uk> [mailto:xerte-dev-bounces at lists.nottingham.ac.uk] On Behalf Of Smith, John
Sent: 08 March 2013 14:12
To: For Xerte technical developers
Subject: [Xerte-dev] Re: SECURITY PATCH for upload.php

Ok I’ll look at that and see why… maybe config isn’t being included properly… sessions work elsewhere in Firefox so why not here?

Regards,

John Smith
Learning Technologist
School of Health & Life Sciences
Glasgow Caledonian University

From: xerte-dev-bounces at lists.nottingham.ac.uk<mailto:xerte-dev-bounces at lists.nottingham.ac.uk> [mailto:xerte-dev-bounces at lists.nottingham.ac.uk] On Behalf Of Julian Tenney
Sent: Friday, March 08, 2013 2:05 PM
To: For Xerte technical developers
Subject: [Xerte-dev] Re: SECURITY PATCH for upload.php

I commented it out because it didn’t work in firefox.

From: xerte-dev-bounces at lists.nottingham.ac.uk<mailto:xerte-dev-bounces at lists.nottingham.ac.uk> [mailto:xerte-dev-bounces at lists.nottingham.ac.uk] On Behalf Of Smith, John
Sent: 08 March 2013 13:55
To: For Xerte technical developers
Subject: [Xerte-dev] Re: SECURITY PATCH for upload.php

If we get pushed for time and we need to then we can just comment out the code I added for now. All it would do then is the session check, although even that check was commented out in the svn and probably the 1.9 release, no idea why though or by who and whether adding that back in will be causing an issue…

Regards,

John Smith
Learning Technologist
School of Health & Life Sciences
Glasgow Caledonian University

From: xerte-dev-bounces at lists.nottingham.ac.uk<mailto:xerte-dev-bounces at lists.nottingham.ac.uk> [mailto:xerte-dev-bounces at lists.nottingham.ac.uk] On Behalf Of Julian Tenney
Sent: Friday, March 08, 2013 1:47 PM
To: For Xerte technical developers
Subject: [Xerte-dev] Re: SECURITY PATCH for upload.php

I’m not sure I’m close enough to the detail to recommend a way forward here, so happy to go with a recommendation, but would like to see it all implemented at once in the svn so we’re not in a position where exporting the svn creates an install that won’t upload anything…

From: xerte-dev-bounces at lists.nottingham.ac.uk<mailto:xerte-dev-bounces at lists.nottingham.ac.uk> [mailto:xerte-dev-bounces at lists.nottingham.ac.uk] On Behalf Of Smith, John
Sent: 08 March 2013 13:26
To: For Xerte technical developers
Subject: [Xerte-dev] Re: SECURITY PATCH for upload.php

I suppose then we could have a script in management that parses the extensions from the xwd files and warns which extensions need to be added to the field in order to make the model fully usable… would that be duplicating and adding to the confusion of having a mime types field in sitedetails too?

Well I’ll work on the basis that I’ll be getting extensions from somewhere and make sure to check session, extension and mime type (based on allowable types for that extension).

One more question, do the error messages (before exit(); ) get fed back to the flash? Should we even include them then as they give a hacker some hint as to why an exploit failed – should it just be “failed”

Regards,

John Smith
Learning Technologist
School of Health & Life Sciences
Glasgow Caledonian University

From: xerte-dev-bounces at lists.nottingham.ac.uk<mailto:xerte-dev-bounces at lists.nottingham.ac.uk> [mailto:xerte-dev-bounces at lists.nottingham.ac.uk] On Behalf Of Julian Tenney
Sent: Friday, March 08, 2013 1:13 PM
To: For Xerte technical developers
Subject: [Xerte-dev] Re: SECURITY PATCH for upload.php

I think my preference would be for a global setting: comma seperated list of allowed types in management.php so we ship a secure product, and people can change the settings if they want to.

From: xerte-dev-bounces at lists.nottingham.ac.uk<mailto:xerte-dev-bounces at lists.nottingham.ac.uk> [mailto:xerte-dev-bounces at lists.nottingham.ac.uk] On Behalf Of Pat @ Pgogy
Sent: 07 March 2013 17:14
To: For Xerte technical developers
Subject: [Xerte-dev] Re: SECURITY PATCH for upload.php

Hello,

Problem is getting a list of types the models support. I think there is a case for saying the model should list the extensions it supports as it is partly their job to do it. This way the model could post the allowed types and the list is generated on the fly?

I agree preventing php is a good thing, but I think the problem is not knowing what types are acceptable is a real curveball

Pgogy Webstuff - http://www.pgogywebstuff.com
Makers of web things of a fair to middling quality

On 7 Mar 2013, at 15:32, "Smith, John" <J.J.Smith at gcu.ac.uk<mailto:J.J.Smith at gcu.ac.uk>> wrote:
Hi Pat,

I didn’t copy your regexp or your select list directly but translated the select code into a comma separated list so that it can be moved elsewhere if required…

I noticed the list in the sitedetails table but it is of Mime Types. I think it would be best practice to use extensions, content headers, mimetypes and any other method available to whitelist the allowable files but I think that might take a bit more work…

I think it is leaving a load of sites out there very vulnerable so we should try to  find a good way to shore this up before the next release. What do you think? I’ll have a go at adding in some code to deal with content headers and mimetypes

Regards,

John Smith
Learning Technologist
School of Health & Life Sciences
Glasgow Caledonian University

From: xerte-dev-bounces at lists.nottingham.ac.uk<mailto:xerte-dev-bounces at lists.nottingham.ac.uk> [mailto:xerte-dev-bounces at lists.nottingham.ac.uk] On Behalf Of Pat @ Pgogy
Sent: Thursday, March 07, 2013 2:54 PM
To: For Xerte technical developers
Subject: [Xerte-dev] Re: SECURITY PATCH for upload.php

Hello,

I hobble the Wordpress version deliberately to only allow a few file types but that isn't the list that full XOT needs (there is in fact, no list, hence the problem).

My reg exp is a bit flaky as well, if you copied that over.

There is a sort of whitelist in the sitedetails table as the media upload properties panel page uses this - but not sure this is the XOT list.

Pgogy Webstuff - http://www.pgogywebstuff.com
Makers of web things of a fair to middling quality

On 7 Mar 2013, at 13:01, "Smith, John" <J.J.Smith at gcu.ac.uk<mailto:J.J.Smith at gcu.ac.uk>> wrote:
Hi,

I’ve just committed a change to upload.php (revision 714) to stop users exploiting a system by uploading php code. I’ve added a whitelist and stuck in the same allowed file extensions that Pat uses in the Wordpress plugins.

Can someone test this and advise if there are any other media types that we want/need to allow?

There was also a session check but exit(); was commented out therefore in an unpatched system ANYONE can post data to upload.php and get some code onto the server. I’ve uncommented this now but does anyone know why it was commented out in the first place?

Regards,

John Smith
Learning Technologist
School of Health & Life Sciences
Glasgow Caledonian University

Glasgow Caledonian University is a registered Scottish charity, number SC021474