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

Extremely long codes can cause underflow errors #12

Closed
drinckes opened this issue Jul 7, 2015 · 6 comments
Closed

Extremely long codes can cause underflow errors #12

drinckes opened this issue Jul 7, 2015 · 6 comments

Comments

@drinckes
Copy link
Contributor

drinckes commented Jul 7, 2015

In a test C++ implementation, long codes were causing segmentation faults. This was due to the grid computation, where the size of the grid squares are computed in degrees latitude and degrees longitude. These start at 1/8000th of a degree, and with each step, divide by 4 (latitude) or 5 (longitude).

Fairly quickly these can get to the minimum values that can be represented as floats and cause underflow errors.

To avoid this happening, I propose to limit the length of OLC codes. This would avoid this problem while still allowing extremely precise codes. 16 digits gives roughly centimeter resolution, and with 32 digits the OLC area is only 7.86E-11 cm wide (this is in the order of a helium atom).

I don't think it matters a great deal what the limit is as long as we define one.

The changes required are:

  • To modify the isValid method to reject codes with more than 32 digits or with more than 24 digits after the separator
  • To limit the encode method to only return codes with a maximum of 32 digits.
@tgulacsi
Copy link
Contributor

go-fuzz found similar problems, for example with codes like

+qqX2975722X888X2975722X888X2975722X988X29qqX2975722X888X2975722X888X2975722X988X29qqX2975722X888X2975722X888X2975722X988X29qqX2975722X88qqX2975722X888X2975722X888X2975722X988X29qqX2975722X888X2975722X888X2975722X988X29qqX2975722X888X2975722X888X2975722X988X29qqX2975722X88qqX2975722X888X2975722X888X2975722X988X29qqX2975722X888X2975722X888X2975722X988X29qqX2975722X888X2975722X888X2975722X988X29qqX2975722X88qqX2975722X888X2975722X888X2975722X988X29qqX2975722X888X2975722X888X2975722X988X29qqX2975722X888X2975722X888X2975722X988X29qqX297572

+qqX2975722X888X2975722X888X2975722X988X29qqX2975722X888X2975722X888X2975722X988X29qqX2975722X888X2975722X888X2975722X988X29qqX2975722X88qqX2975722X888X2975722X888X2975722X988X29qqX2975722X888X2975722X888X2975722X988X29qqX2975722X888X2975722X888X2975722X988X29qqX2975722X88qqX2975722X888X2975722X888X2975722X988X29qqX2975722X888X2975722X888X2975722X988X29qqX2975722X888X2975722X888X2975722X988X29qqX2975722X88qqX2975722X888X2975722X888X2975722X988X29qqX2975722X888X2975722X888X2975722X988X29qqX2975722X888X2975722X888X2975722X988X29qqX297572

@drinckes
Copy link
Contributor Author

What I'll probably do is add a test case with a code > 32 digits that should validate to false. Then everyone gets to run their tests. :-)

@drinckes drinckes self-assigned this Aug 21, 2015
@drinckes drinckes added the bug label Aug 21, 2015
@zongweil
Copy link
Contributor

@drinckes is this still applicable?

@drinckes
Copy link
Contributor Author

Probably - to languages other than C++. I think @tgulacsi is pointing out that Go is also having underflow problems without a max length.

@drinckes drinckes removed their assignment May 29, 2018
@zongweil
Copy link
Contributor

As discussed in this thread, we are going to internally limit the number of characters that we process. This will avoid underflow errors.

It will take a while to update all the libraries.

@zongweil
Copy link
Contributor

I'm going to close this bug now that we have a solution going forward. #190 tracks the work of updating all the libraries.

@slpat slpat mentioned this issue Jun 26, 2023
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

No branches or pull requests

4 participants
@tgulacsi @zongweil @drinckes and others