[SLD] crash when the number of filter elements in big

[assefa]

A crash happen with the following sld


<StyledLayerDescriptor version="1.0.0">    <NamedLayer>        
<Name>land_fn</Name>        <UserStyle>            
<FeatureTypeStyle>                <Rule>                    <Filter>
<Or>
<Or>
<Or>
<Or>
<Or>
<Or>
<Or>
<Or>
<Or>
<Or>
<Or>
<Or>
<Or>
<Or>
<Or>
<Or>
<Or>
<Or>
<PropertyIsEqualTo>
<PropertyName>IMAGE_ID</PropertyName>
<Literal>4341</Literal>
</PropertyIsEqualTo>
<PropertyIsEqualTo>
<PropertyName>IMAGE_ID</PropertyName>
<Literal>4391</Literal>
</PropertyIsEqualTo>
</Or>
<PropertyIsEqualTo>
<PropertyName>IMAGE_ID</PropertyName>
<Literal>4392</Literal>
</PropertyIsEqualTo>
</Or>
<PropertyIsEqualTo>
<PropertyName>IMAGE_ID</PropertyName>
<Literal>4438</Literal>
</PropertyIsEqualTo>
</Or>
<PropertyIsEqualTo>
<PropertyName>IMAGE_ID</PropertyName>
<Literal>4439</Literal>
</PropertyIsEqualTo>
</Or>
<PropertyIsEqualTo>
<PropertyName>IMAGE_ID</PropertyName>
<Literal>4440</Literal>
</PropertyIsEqualTo>
</Or>
<PropertyIsEqualTo>
<PropertyName>IMAGE_ID</PropertyName>
<Literal>4484</Literal>
</PropertyIsEqualTo>
</Or>
<PropertyIsEqualTo>
<PropertyName>IMAGE_ID</PropertyName>
<Literal>4485</Literal>
</PropertyIsEqualTo>
</Or>
<PropertyIsEqualTo>
<PropertyName>IMAGE_ID</PropertyName>
<Literal>4486</Literal>
</PropertyIsEqualTo>
</Or>
<PropertyIsEqualTo>
<PropertyName>IMAGE_ID</PropertyName>
<Literal>4487</Literal>
</PropertyIsEqualTo>
</Or>
<PropertyIsEqualTo>
<PropertyName>IMAGE_ID</PropertyName>
<Literal>4528</Literal>
</PropertyIsEqualTo>
</Or>
<PropertyIsEqualTo>
<PropertyName>IMAGE_ID</PropertyName>
<Literal>4529</Literal>
</PropertyIsEqualTo>
</Or>
<PropertyIsEqualTo>
<PropertyName>IMAGE_ID</PropertyName>
<Literal>4530</Literal>
</PropertyIsEqualTo>
</Or>
<PropertyIsEqualTo>
<PropertyName>IMAGE_ID</PropertyName>
<Literal>4570</Literal>
</PropertyIsEqualTo>
</Or>
<PropertyIsEqualTo>
<PropertyName>IMAGE_ID</PropertyName>
<Literal>4571</Literal>
</PropertyIsEqualTo>
</Or>
<PropertyIsEqualTo>
<PropertyName>IMAGE_ID</PropertyName>
<Literal>4611</Literal>
</PropertyIsEqualTo>
</Or>
<PropertyIsEqualTo>
<PropertyName>IMAGE_ID</PropertyName>
<Literal>4612</Literal>
</PropertyIsEqualTo>
</Or>
<PropertyIsEqualTo>
<PropertyName>IMAGE_ID</PropertyName>
<Literal>4650</Literal>
</PropertyIsEqualTo>
</Or>
<PropertyIsEqualTo>
<PropertyName>IMAGE_ID</PropertyName>
<Literal>1000</Literal>
</PropertyIsEqualTo>
</Or>
</Filter>                    <PolygonSymbolizer>                        
<Fill>                            <CssParameter 
name="fill">#BBCCEE</CssParameter>                        
</Fill>                        <Stroke>                            
<CssParameter name="stroke">#FFFF00</CssParameter>                            
<CssParameter name="stroke-width">3</CssParameter>                        
</Stroke>                    </PolygonSymbolizer>                    
<TextSymbolizer>                        
<Label>PUBFNAME</Label>                        
<Font>                            <CssParameter name="font-
family">Arial</CssParameter>                            <CssParameter 
name="font-weight">bold</CssParameter>                            
<CssParameter name="font-size">10</CssParameter>                        
</Font>                        <Fill>                            <CssParameter 
name="fill">#000000</CssParameter>                        
</Fill>                    
</TextSymbolizer>                                      </Rule>            
</FeatureTypeStyle>        </UserStyle>    </NamedLayer>
</StyledLayerDescriptor>

[assefa]

Double the size of static variables used to parse the sld filter.

[sgillies@…]

Assefa, is this a security problem? Buffer overflows are dangerous. Doubling the
buffer sizes doesn't seem to be a good enough solution: a script kiddie could
just double the size of his huge bogus SLD and take mapserver down anyway.

[dmorissette]

I second Sean's comment, static buffers with no proper bounds validation are evil.

[assefa]

Update to use dynamic allocation.

Will backport this to 4.6 branch after furthur testing.

[sgillies@…]

there should probably be an announcement to users? 

[assefa]

Do you see the annoucement due to the fact that It will be backported to the 
stable version or as a critical bug correction that the users should have ? 
Not sure what the policy should be when a bug is corrected but I did not see a 
lot of e-mails in the last years annocuing a bug correction.  

[dmorissette]

Ouch! mid-air collision... I'll submit my comment anyway even if it's repeating
some of what Assefa wrote already...


I dunno. Why do you see a need for an announcement? Is it that you see this as a
security vulnerability? Should we really do an announcement everytime we fix a
potential buffer overrun or a potential crash even if there is no evidence that
it can be exploited?

In this case, a DoS on mapserv is unlikely since it's a CGI so a crash of a
malformed request has no impact on legitimate requests. The other threat I could
imagine is that a properly forged SLD could be used to corrupt the stack and
exploit a server, but we have no evidence that such an exploit could be built.

Am I missing a more serious issue?

[sgillies@…]

I don't think the exploit would be any harder than the exploits of the past (see
BugTraq and CERT pages for history). Google "smashing the stack for fun and
profit" for a great how-to. It's more a matter of whether anyone cares to do so,
and how many of our users run Apache with escalated privileges. Hopefully, they
do not.

I am definitely not going to sound an alarm without consensus and support from
you two. If you don't think it's a big deal, I'll let it go.

[banders@…]

The dynamic allocation fix seems to work for me with mapserver 4.6.1.  I tested
with a SLD that has hundreds of <Or> expressions -- a much larger filter than
the sample above -- and there were no problems.

Which branches will this patch be applied to?

[assefa]

I have commied it into the  branch-4-6. It shoudl be abailable for the 4.6.2 
release.

I have cc'd Tom in this bug. Tom if you got time please run a test with you 
data. 

[tomkralidis]

Sure, I run latest CVS.  Can I test against latest CVS?

[assefa]

Thanks Tom. Please do the tests aginst latest cvs in the branch-4-6. (the 
4.6.2 to be)

[assefa]

Closing bug.