Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Catch over range lam0 in proj_etmerc.c #899

Merged
merged 1 commit into from Mar 25, 2018

Conversation

schwehr
Copy link
Member

@schwehr schwehr commented Mar 25, 2018

lam0 of inf caused a nan in the cast to int.
Picked +/-1000 for lam0 as a guess for what constitutes reduculously large values.

proj_etmerc.c:361:16: runtime error: nan is outside the range of representable values of type 'int'

Found with autofuzz: UndefinedBehaviorSanitizer: float-cast-overflow

@schwehr
Copy link
Member Author

schwehr commented Mar 25, 2018

poc for gdal/autotest2/cpp/ogr:ogr_srs_proj4_fuzzer:

+proj=pipeline	++proj=utm +lon_0=7e902
+stex	+step	+ste

@kbevers
Copy link
Member

kbevers commented Mar 25, 2018

The fix seems find. Thanks for catching the problem. May I ask though that you move the P->lam0 check up ~15 lines, below the P->es check, so that the error checking is done before entering the main logic of the code? We[0] try to follow that principle in new code. The reason being that you might as well fail as early as possible and save some CPU cycles and that it removes unnecessary mental obstructions when reading the code since the algorithm is written in it's pure form without any detours.

[0] at least @busstoptaktik and me

lam0 of inf caused a nan in the cast to int.
Picked +/-1000 for lam0 as a guess for what constitutes reduculously large values.

proj_etmerc.c:361:16: runtime error: nan is outside the range of representable values of type 'int'

Found with autofuzz: UndefinedBehaviorSanitizer: float-cast-overflow
@schwehr schwehr force-pushed the proj_etmerc-inf-zone-b76304619 branch from 55bba97 to e52ab92 Compare March 25, 2018 20:52
@schwehr
Copy link
Member Author

schwehr commented Mar 25, 2018

I'm of the same mindset... range check up front as much as possible.

Hopefully this redo of the patch doesn't have any surprises. I don't know proj well enough to have a sense of when something might have a surprise side effect.

@kbevers
Copy link
Member

kbevers commented Mar 25, 2018

This should be fine. Generally speaking, when inside a PJ_*.c file you will only be able to cause side effects in code within that file.

@kbevers kbevers merged commit 970bad5 into OSGeo:master Mar 25, 2018
@kbevers kbevers added this to the 5.1.0 milestone Mar 25, 2018
@schwehr schwehr deleted the proj_etmerc-inf-zone-b76304619 branch March 25, 2018 21:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants