Memory error in postgis provider

[strk]

Trunk svn 9801.

==20521== Invalid write of size 1
==20521==    at 0x4E67BC4: _IO_default_xsputn (in /lib/libc-2.9.so)
==20521==    by 0x4E3E73C: vfprintf (in /lib/libc-2.9.so)
==20521==    by 0x4E5C1EB: vsprintf (in /lib/libc-2.9.so)
==20521==    by 0x4E44EFA: sprintf (in /lib/libc-2.9.so)
==20521==    by 0x814CFC0: msPostGISBuildSQLBox (in /net/lafont/home/santisa/src/mapserver/trunk/map
server/mapserv)
==20521==  Address 0x6bcb8c3 is 0 bytes after a block of size 243 alloc'd
==20521==    at 0x4027DDE: malloc (in /usr/lib/valgrind/x86-linux/vgpreload_memcheck.so)
==20521==    by 0x814CF62: msPostGISBuildSQLBox (in /net/lafont/home/santisa/src/mapserver/trunk/map
server/mapserv)

[strk]

Problem is that the size of the allocated buffer is computed by *assuming* 15 characters for doubles printed with %.15g, but that format requests 15 *significant* digits, not 15 digits in general. Using %.15f would fix that.

[strk]

Limit number of digits to 15, no matter how many significant. Make sure not to write out of allocated buffer by using snprintf rather than sprintf

[strk]

The attached path makes the code more robust by using snprintf, warns on truncation, uses %f format to get predictable number of digits in output.

[pramsey]

Patch applied to 5.4, 5.6 and trunk

[dmorissette]

Reopening.

I have received a report that 5.6.2 (which includes the fix above) breaks with the following error with a mode=map request with mapext=2701967.5295031+1184936.875+2705055.4704969+1186876.375

msDrawMap(): Image handling error. Failed to draw layer named 'ownershipqpoly'.
msPostGISLayerWhichShapes(): Query error. Failed to build query SQL.
msPostGISBuildSQL(): General error message. Failed to build SQL 'where'.
msPostGISBuildSQLWhere(): General error message. Unable to build box SQL.
msPostGISBuildSQLBox: General error message. Bounding box digits truncated. 

The "%.15f" format does NOT guarantee a total of 15 digits, it requests 15 digits AFTER the decimal point... so in most cases you are guaranteed to get more than 15 digits e.g.

printf("%.15f\n%.15f\n%.15f\n%.15f\n", 2701967.5295031, 1184936.875, 2705055.4704969, 1186876.375);

2701967.529503100086004
1184936.875000000000000
2705055.470496899913996
1186876.375000000000000

I think the right fix would be to use the %.15g format and account a few extra chars per coordinate for the potential use of exponential notation.

[dmorissette]

With the %.15g format, the worst case is the exponent notation which is:

  [-]m.dddde[+/-]xxx

Based on that the total number of chars should be 15 + 1 for the optional sign, + 1 for the dot, and + up to 5 for the exponent ... so that's up to 22 chars per number for doubles. e.g.

printf("%.15g\n", -1.234567890123456789e300);

-1.23456789012346e+300

[dmorissette]

Patch that reverts back to %.15g and tests for 22 chars per value

[dmorissette]

Richard, can you please test the new patch and confirm that it fixes the issue with your test case?

[dmorissette]

Richard has confirmed that the patch to use %.15g and tests for 22 chars per value fixes the issue for him so I have applied it in branch-5-6 r9967 and branch-5-4 r9868.

I see that theduckylittle had already increased the size from 15 to 26 in r9866 (a larger commit for SVG symbol stuff), but had kept the format to %.15f. Unfortunately I don't think that was enough to guard us against possible overflows e.g.

    printf("%.15f\n",1e30);

1000000000000000019884624838656.000000000000000

So I have reverted his change and applied the same patch to use use %.15g and tests for 22 chars per value in SVN trunk r9969.

Hopefully that will cover all cases. If anyone can think of a case that the new patch does not cover then please let me know ASAP before I release 5.6.3.