I see a few cases of being able to run off the stack during sprintf.
I'd prefer all the sprintfs were checked and replaced with snprintf, but
it looks to be a large task:
$ grep -R sprintf . | wc -l
472
As long as this compiles without warnings from -Wformat-security and
compiles with the now-default -D_FORTIFY_SOURCE=2 at -O2, we should get
the libc protections for most abuse cases of *printf. (i.e. it will
need a no-change rebuild bump to get recompiled with the hardened
toolchain). (Also -- it does compile fine, I just ran a rebuild.)
scanf seems to be accidentally safe -- old BUFF_SIZE was 1024, which was
raised. Some of the sscanf's where %1024s, which would have been a
problem. The rest are okay.
One area that bothers me is the unbounded mallocs using multiplication and
user-controlled input. For example in src/formats/xtcformat.cpp:
xdr_int(&xd, &natoms);
...
floatCoord = (float *)malloc(natoms * 3 * sizeof(float));
...
// Read the positions
if (xdr3dfcoord(&xd, (float *)floatCoord, &natoms, &prec) == 0) {
There really needs to be bounds checking for this stuff to keep
allocations from wrapping. Prior to that happening I would need to
recommend against it going into main.
$ grep -R 'alloc(.*\*' . | wc -l
157
** Changed in: openbabel (Ubuntu)
Assignee: Ubuntu Security Team (ubuntu-security) => Jonathan Riddell (jr)
--
main inclusion review for openbabel
https://bugs.launchpad.net/bugs/236051
You received this bug notification because you are a member of Ubuntu
Bugs, which is subscribed to Ubuntu.
--
ubuntu-bugs mailing list
ubuntu-bugs@lists.ubuntu.com
https://lists.ubuntu.com/mailman/listinfo/ubuntu-bugs
