Hi, On Sun, Dec 23, 2012 at 01:11:22PM +0100, Arne Schwabe wrote: > I fear we may have to delay the final release. While debugging a problem > with a user of my app I found a nasty bug which causes OpenVPN to crash > if two PUSH_CONTROL messages are received. > > from push.c: > > if (!c->c2.did_pre_pull_restore) > { > pre_pull_restore (&c->options); > md5_state_init (&c->c2.pulled_options_state); > c->c2.did_pre_pull_restore = true; > } > > This is executed on the first push message and initializes the md5 > state. On further push messages (continuation or new) this is not > executed again.
Indeed, this is coupling two things - "pre-pull options saving" (to restore to the state before pulling, which only ever needs to be executed once), and "md5 state init", which needs to be executed for every round of PUSH_REPLY... Assuming there is only one PUSH_REPLY ever, this is no problem, but with the new server side code plus mobile networks, this can indeed be triggered. I'd propose to fix this by adding "bool c->c2.pulled_options_md5_init", using that for md5_state_init() *and resetting it* after md5_state_final(). For a single PUSH_REPLY, this should not make a difference (except for a few bytes of code), and for multiple PUSH_REPLY messages this will re-init the md5_state properly. (I'll send a patch as soon as I can test this - Arne, could you send me login credentials + config for your "will crash client!" server, please?) gert -- USENET is *not* the non-clickable part of WWW! //www.muc.de/~gert/ Gert Doering - Munich, Germany g...@greenie.muc.de fax: +49-89-35655025 g...@net.informatik.tu-muenchen.de
pgpSRjWVM3wZK.pgp
Description: PGP signature