search

coin-or / ADOL-C

A Package for Automatic Differentiation of Algorithms Written in C/C++
Other
146 stars 31 forks source link

container-overflow and double free in `cleanUp tape_handling.cpp:588` #81

Open michaelvanstraten opened 6 days ago

michaelvanstraten commented 6 days ago

The issue arises because a reference to the last element is recorded at https://github.com/coin-or/ADOL-C/blob/d5e3c2a3573ae6c5c92a882c13bc6f1714cae21f/ADOL-C/src/tape_handling.cpp#L584 followed by an immediate resize of the container to exclude that element at https://github.com/coin-or/ADOL-C/blob/d5e3c2a3573ae6c5c92a882c13bc6f1714cae21f/ADOL-C/src/tape_handling.cpp#L585

This behavior results in a container-overflow.

Additionally, the call at https://github.com/coin-or/ADOL-C/blob/d5e3c2a3573ae6c5c92a882c13bc6f1714cae21f/ADOL-C/src/tape_handling.cpp#L678 could potentially result in a double free if the container is deallocated before the program ends. Fortunately, this does not occur in this case because the container is global.

Given how this function is implemented, it seems unusual to use the std::vector class (reference) without leveraging its destructor for automatic cleanup tasks like closing file handles. This design choice appears suboptimal, especially since it could be improved for better compatibility with modern C++ standards.

Fortunately, this issue can be resolved easily by moving the pop_back() operation to the end of the deinitialization process.

Please let me know if you'd like me to refactor the class to handle file closures and other cleanup tasks automatically via the destructor. This would align the implementation with best practices and enhance maintainability.

TimSiebert1 commented 5 days ago

Hi @michaelvanstraten,

thanks that you point that out. For the moment I would not like to refactor this code, since we have to start with some stock-taking to see the interactions between files and functionalities in the files, what is testet, what should be replace and so on. After that we slowly move towards a new version of ADOL-C and you are definitely invited to participate here. If you want to help in the stock-tacking process. Let me know!