search

opnsense / dhcp6c

OPNsense WIDE-DHCPv6 client
Other
22 stars 30 forks source link

segfault #8

Closed BrainSlayer closed 5 years ago

BrainSlayer commented 5 years ago

a 100% segfault on big endian is the missuse of a long long number for pointers in the config parser for the rawoption option number. this will crash on and mips big endian system. use "long" or use a string or even use a numeric value. but never use 64 bit integers as casted pointer here.

marjohn56 commented 5 years ago

Can you point out where this is in the code, I've looked and failed to see- not unusual!

BrainSlayer commented 5 years ago

its easy. the yacc/bison generators uses long long integers for integers but its a string pointer in reality an this integer is casted to a pointer later again. this fails on big endian 32 bit systems of course . it turns to zero. i fixed that already in the dd-wrt repository

marjohn56 commented 5 years ago

Probably best if you issue a PR for it. You can discuss with Franco. :)

marjohn56 commented 5 years ago

Calling him in shout out to @fichtner

fichtner commented 5 years ago

we do not have big endian builds nor lack of politeness over here, so we are fine

BrainSlayer commented 5 years ago

my solution is a quick and dirty workaround. the problem isnt solved. pointer should not be casted to long long integers and back to pointers in any way. long long is 64 bit and pointers can be 32 bit or 64 bit depending on the system and big and little endian fucks it up at the end if you try to cast them. my solution is that i fixed all integers to long instead of long long. so the datatype fits to the system at the end

BrainSlayer commented 5 years ago

@fichtner : thats the worst answer i've ever seen. in embedded systems where dhcp6c is used widely big endian systems are common. mips for instance. or xscale. i know you made it with intension to be used for freebsd. but since patches like custom options (-x) are getting important for routers as well to support strange isp's i decided to use this version here for integration into dd-wrt. or at least some patches of it. so i found out that the code is not correct at all and it no excuse to say that you never have seen a problem with it since your target isnt big endian. other targets might be. even with freebsd. and its simple to fix. the current way of handling pointers for the config parser is per definition incorrect. long long cannot be casted to pointers. since the length of a pointer is undefined depending on the architecture, but the size of long long is fixed to 64 bit

fichtner commented 5 years ago

@BrainSlayer you will be blocked if you keep indicating how bad we are and how much all of this sucks, and if you want to prove how clever you are just open a PR so we can talk review :)