On 30/06/18(Sat) 23:47, David Bern wrote: > > Note that your diff doesn't apply. You have tab vs spaces issues. > Sorry about that, it seems like I need to setup a proper mail-client. > > > I don't understand what you're talking about. Can you give example of > > the 3 scenarios you're talking about. > If you look inside /usr/share/misc/usb_hid_usages you will find it in > 9 Button, 10 Ordinal, 16 Unicode and 129 Monitor Enumerated Values. > > > There's also a similar fix in NetBSD's tree, did you look at it? > I took a look now and it enlightened a potential problem. > > NetBSD is using fmtcheck to make sure a "correct" format string is > used. I could perhaps make an effort on a patch adding that function into > stdio in OpenBSD, as that solves uses of %u compatible format strings > https://man.openbsd.org/NetBSD-7.1/fmtcheck.3
Better fix the current problem without adding a new function. > I do now also see what your concerns about scanf might of been. > My proposed patch open up for a two step "attack". > If a malicious user is able to alter the contents of usb_hid_usages > and change the name to add something like %s and if a program then > lets a user specify a name to be used by hid_parse_usage_in_page > it could then perform a buffer overflow. > > My first solution of using a "static" format-string would not been affected > by this potential attack or the version using strtonum(). but at the same > time > it would not be able to parse the 16 Unicode example. Is it necessary to parse these examples? Or maybe we can live with your strtonum() fix for now. > One middle way solution could be to write a "scanf" that only handles %u or > %x > format strings, in other words a simplified variant of fmtcheck. > > > Would really love to get some advice before continuing working on this > patch. I don't know. So unless somebody else gives us some input I'd suggest we move forward with your safe diff even if it doesn't fix all the cases. If it fixes your use case it is already an improvement.
