On Mon, May 19, 2014 at 11:55:33PM +0200, Jérémie Courrèges-Anglas wrote: > > It is not about this particular occurrence. If you replace it by > a strlcpy call, you will probably see the same warning, coming from > another strcpy call elsewhere. > > Propose upstream strl* and snprintf as alternatives to the unchecked > functions. If upstream sees interest in this, then a careful audit of > the code can follow, and strl* functions make error checking easier. > Adding fallback code for OSes not providing those functions is easy too. >
Upon mentioning these findings I was informed of the situation where an ongoing rewrite of the code for 2.0 will make sure strcpy/strcat is not used, so this will fix itself in time. It is available here: http://lists.opendnssec.org/pipermail/opendnssec-user/2014-May/002941.html > > > > The call in signer/src/wire/notify.c: > > random()%(extra-base); > > > > I guess #ifdef'ing this with HAVE_ARC4RANDOM like is done in ksm_policy.c > > would be nice. > > See arc4random_uniform(3), the configure seems to test for its presence. > The current random-related fixes on the upstream openbsd branch actually tests for both now. Thanks for the pointer! > > > Finally some unused variables, might as well be complete while I am at > > it: > > > > === > > daemon/cmdhandler.c:398: warning: unused variable 'task' > > > > shared/duration.c:442: warning: 'is_leap_year' defined but not used > > shared/duration.c:449: warning: 'leap_days' defined but not used > > > > shared/hsm.c:224: warning: unused variable 'retries' > > shared/hsm.c:220: warning: unused variable 'status' > > === > > Those could be harmless, or they could show a bug in the program logic. > If you want to audit them fine, but in the end it's rather upstream's > responsibility. > These was mostly intended to be seen by upstream, and some of them has been fixed on the branch. > We try to avoid having too much patches in the ports tree. The best way > is probably to work with upstream and get your patches integrated, after > proper reviewing. Some mistakes have been done while wrongly replacing > eg. strcpy by strlcpy. > I intend to ignore the strcat/strcpy stuff for now given the above status. The other stuff has been fixed. Thanks a lot for your input :). Regards, Patrik Lundin
