search

cyjoelchen / php-sweph

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

Issue/36 #42

Closed aloistr closed 3 years ago

aloistr commented 3 years ago

ready for merge

aloistr commented 3 years ago

I used compiler option -Wall which should be turned on by default, to discover many of these issues.

kevindecapite commented 3 years ago

Yes, I ran into this when originally tackling the problem with swe_rise_trans() not parsing the value of ipl correctly. I needed to use size_t instead of int. I failed to notice that in this debug session.

But size_t is 64 bit, 8 byte, and if a string length parameter is declared only as int, a memory overwrite can happen

It is good to now know why this issue occurs.

aloistr commented 3 years ago

Yes, I ran into this when originally tackling the problem with swe_rise_trans() not parsing the value of ipl correctly. I needed to use size_t instead of int. I failed to notice that in this debug session.

But size_t is 64 bit, 8 byte, and if a string length parameter is declared only as int, a memory overwrite can happen

It is good to now know why this issue occurs.

Actually, long is 64 bit, and so it makes a difference whether long or int are used. long is essential. size_t is also 64 bit.

It is important that the programmer checks the parameter list carefully. I was not aware of the issue when I added new functions, and made errors. Numerous errors were also in the older code. I have now checked the parameter list for all calls of zend_parse_parameters().