openvpn --setenv opt segfaults

[Selva Nair]

openvpn --setenv opt without a second argument segfaults on Linux and Windows and may be other platforms.

version 2.4_beta2 (tagged as alpha2 as I cant find beta2 in the drop down menu).

P.S.
It seems older version segfaults too, so this is not 2.4-specific.

[Gert Döring]

Indeed, we broke that somewhen in 2.3...

gert@fbsd74:/root/t_server$ ./openvpn.22 --setenv opt
Options error: You must define TUN/TAP device (--dev)
Use --help for more information.
gert@fbsd74:/root/t_server$ ./openvpn.23 --setenv opt
Memory fault
gert@fbsd74:/root/t_server$ ./openvpn.master --setenv opt
Memory fault

Since this cannot be pushed (phew!)...

Tue Nov 29 21:21:55 2016 Options error: option 'setenv' cannot be used in this context ([PUSH-OPTIONS])

... this is not something that "must! absolutely!" be fixed for 2.4 release... but it's good you found it. Segfaults are always embarrassing... wonder who broke that :-)

[Selva Nair]

2.2 probably did not have setenv-opt. The first commit seems to be
commit 2a92fba756d4c1e73300a12ff9e80028a6ab7c09
Author: James Yonan <james@…>
Date: Tue Jun 11 00:25:05 2013 -0600

which is apparently cherry-picked from commit 27713761e4110bb92f1c6dfe85db291e8c6e0f56 but that commit is not in my tree..

Anyway, bikeshedding here as this is not critical. The fix is easy though:

if (streq (p[0], "setenv") && p[1] && streq (p[1], "opt") && !(permission_mask & OPT_P_PULL_MODE))
  {
      p += 2;
      msglevel_fc = M_WARN;
  }

should check that p[2] is not null before doing p += 2. Else the new options head, p[0], will be NULL and get dereferenced in subsequent parsing using streq()'s.

[Gert Döring]

commit 997795353916ffcb413a2da02dc7f210fd621954 (master)
commit 290cc3f8d50435a6ed5f2cb1ecd9056dadcc4783 (release/2.3)
Author: Selva Nair
Date: Tue Nov 29 20:53:14 2016 -0500

When parsing '--setenv opt xx ..' make sure a third parameter is present

thanks :)