Davy Chan wrote:
Hello Humberto,
I applaud your effort but the use of dynamically allocated buffers
verse statically allocated buffers does not alleviate the buffer-overflow
problem. It mitigates trash-writing to somewhere else. Additionally, using
dynamically allocated buffers does slow the system down and, potentially,
generate a Kannel PANIC.
Also, some of the code you pointed to uses information from the
Kannel configuration file (e.g. the serial device filename for the
emi_x25). I feel we should document the length limitation in the
userguide and let the person configuration shoot himself/herself
in the foot if they go beyond it. If we are kind, then we can provide
'%s' precision to limit their inputted value to conform to the
stated acceptable length.
But, I do agree with you in the code sections were the information is
coming from outside our control. For those sections, we should use
'%s' precision and maybe even snprintf() for that "belt-and-suspender"
feeling.
Did you get your ideas for the changes through an automated
buffer-overflow checking application? It appears that many of the
suggestions didn't take into effect the coding around the proposed
fix.
More comments inline...
agree'ing to Davy partly.
I have addressed these symptoms also in a private branch. This needs to be
cleaned down and commited.
How far has the processing logic been considered in the patch?
@Davy, you seem to picked-up this. Can you go for a cleanup round and provide a
clean patch to commit for?
Stipe
-------------------------------------------------------------------
Kölner Landstrasse 419
40589 Düsseldorf, NRW, Germany
tolj.org system architecture Kannel Software Foundation (KSF)
http://www.tolj.org/ http://www.kannel.org/
mailto:st_{at}_tolj.org mailto:stolj_{at}_kannel.org
-------------------------------------------------------------------
