search

cyjoelchen / php-sweph

PHP Extension for Swiss Ephemeris
Other
63 stars 29 forks source link

string overflow bug in fixstar functions #69

Closed aloistr closed 3 years ago

aloistr commented 3 years ago

as astorigin pointed out, there is a string overflow situation in swephp.c swe_fixstar* functions. His proposed solution has its own problems. It copies data which do not exist and can lead to memory access errors.

The SE functions allow for star names up to 40 characters, plus a null terminator. The constant SE_MAX_STNAME = 256 as defined in swephexp.h is far to large and misleading. There exists an internal constant SWI_STAR_LENGTH = 40 which should be used. I will create a branch issues/69 with a fix.

aloistr commented 3 years ago

We are safe from the php side now.

I will also check the upstream C code in sweph.c and swecl.c to no buffer overrun can happen, and I will change the definition of SE_MAX_STNAME in the next release to 40.

aloistr commented 3 years ago

I cleaned up the handling of star names in swe_fixstar* One can rely on the fact that zend_parse_parameters null-terminates the returned strings.

astrorigin commented 3 years ago

Thank you. I had always thought 256 was probably way too much, but what do I know!

aloistr commented 3 years ago

I fixed another potential bug in swephp.c. In functions swe_sol_eclipse_where() and swe_lun_occult_where() the C code contains this code for (i = 0; i < 10; i++) geopos[i] = 0; This can lead to crashes if array geopos[] is smaller than 10. It was not well documented in SE documentation, so far. This will change soon.

kevindecapite commented 3 years ago

I now see the confusion between the SE docs and the actual return values. For instance, I experimented by adding 10 doubles to the geopos array to see if additional data would be returned, but indexes 2-9 were all "0", despite the docs indicating there are additional values there.

Not a big deal of course, I was only curious to see what would happen.