Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SEGV from unsigned integer overflow in Conformer::setAtomPos #4128

Closed
hgarrereyn opened this issue May 11, 2021 · 0 comments
Closed

SEGV from unsigned integer overflow in Conformer::setAtomPos #4128

hgarrereyn opened this issue May 11, 2021 · 0 comments
Assignees
Labels
Milestone

Comments

@hgarrereyn
Copy link

Describe the bug
There is an unsigned integer overflow bug in Conformer::setAtomPos:

inline void setAtomPos(unsigned int atomId, const RDGeom::Point3D &position) {
  if (atomId >= d_positions.size()) {
    d_positions.resize(atomId + 1, RDGeom::Point3D(0.0, 0.0, 0.0));
  }
  d_positions[atomId] = position;
}

atomId + 1 can overflow in the argument to d_positions.resize.

To Reproduce
This is reproducible in C++ and Python. The index 0xffffffff overflows and Conformer::setAtomPos calls d_positions.resize(0,...) which succeeds.

#include <Geometry/point.h>
#include <GraphMol/Conformer.h>

int main() {
    RDKit::Conformer *conf = new RDKit::Conformer();
    RDGeom::Point3D *point = new RDGeom::Point3D();
    conf->setAtomPos(0xffffffff, *point);
}
import rdkit

conf = rdkit.Chem.Conformer()
point = rdkit.Geometry.Point3D()
conf.SetAtomPosition(0xffffffff, point)

Expected behavior
This should fail with an out-of-bounds error message.

Configuration (please complete the following information):

  • RDKit version: current master branch (92b52f1) but should be the same on 2021.03.01
  • OS: Ubuntu 18.04
  • Python version (if relevant): 3.9.2 (but will work in any version)

Additional context
The C++ example produces the following ASAN report:

==20==ERROR: AddressSanitizer: SEGV on unknown address 0x00047fff7ffd (pc 0x00000051aeff bp 0x7ffcb0bfd450 sp 0x7ffcb0bfd3e0 T0)
==20==The signal is caused by a READ memory access.
    #0 0x51aefe in RDGeom::Point3D::operator=(RDGeom::Point3D const&) /src/rdkit/Code/Geometry/point.h:98:7
    #1 0x518818 in RDKit::Conformer::setAtomPos(unsigned int, RDGeom::Point3D const&) /src/rdkit/Code/GraphMol/Conformer.h:111:25
    #2 0x518347 in main poc.cpp:7:11
    #3 0x7f06e76e8bf6 in __libc_start_main /build/glibc-S9d2JN/glibc-2.27/csu/../csu/libc-start.c:310
    #4 0x41ba79 in _start (poc+0x41ba79)
@hgarrereyn hgarrereyn added the bug label May 11, 2021
@greglandrum greglandrum added this to the 2021_03_3 milestone May 25, 2021
@greglandrum greglandrum self-assigned this May 25, 2021
greglandrum added a commit to greglandrum/rdkit that referenced this issue May 25, 2021
greglandrum added a commit that referenced this issue Jun 9, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants