[PATCH] udhcpc: fix segmentation fault on empty bin opt

Michał Kazior kazikcz at gmail.com
Thu Sep 26 09:47:02 UTC 2019


On Wed, 25 Sep 2019 at 14:28, Tito <farmatito at tiscali.it> wrote:
> On 9/25/19 2:03 PM, Michal Kazior wrote:
> > From: Michal Kazior <michal at plume.com>
> >
> > The following caused udhcpc to segfault:
> >    busybox udhcpc -i lo -s /dev/null -x 0x3d:
> >
> > Signed-off-by: Michal Kazior <michal at plume.com>
> > ---
> >   networking/udhcp/common.c | 2 +-
> >   1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/networking/udhcp/common.c b/networking/udhcp/common.c
> > index 4a452cdb9..9ec752dfc 100644
> > --- a/networking/udhcp/common.c
> > +++ b/networking/udhcp/common.c
> > @@ -539,7 +539,7 @@ int FAST_FUNC udhcp_str2optset(const char *const_str, void *arg,
> >
> >               if (optflag->flags == OPTION_BIN) {
> >                       val = strtok(NULL, ""); /* do not split "'q w e'" */
> > -                     trim(val);
> > +                     if (val) trim(val);
> >               } else
> >                       val = strtok(NULL, ", \t");
> >               if (!val)
> >
>
> Hi,
>
> wouldn't it make more sense to put this in libbb/trim.c so it is fixed definitely?

I'm a little uneasy to patch trim(). It seems convenient, but it also
gives an impression that perhaps other functions should be defensively
NULL checked as well, which not many of them seem to be (if any)
today. It'd end up with inconsistent function behavior with regards to
NULL handling. And you'd still need to do NULL checks afterwards
anyway (either the argument, or return value).

While perhaps not the best example, libc doesn't try to be too smart
about NULL checks for string functions and leaves that to the caller.
trim() seems to be intended as libc-like primitive for string
manipulation.

I'm neither in favor nor against at this point. I don't know what the
design principles are for busybox well enough. I can resubmit a v2 if
you wish.


Michal


More information about the busybox mailing list