Asan doesn't catch basic FMR (Free Memory Read)

[naveendugar1010]

#include <iostream>
#include <set>
using namespace std;

void DeleteElements(set<int> &mp) {
  if (mp.empty())
    return;
  for (auto it: mp) {
    if (it%2) {
      mp.erase(it);
    }
  }
}
int main() {
  std::set<int> mp;
  for (int i = 0; i < 12; i++) {
    mp.insert(i);
  }
  DeleteElements(mp);
  for (auto& it: mp) {
    cout << it << "\n";
  }
}

g++ map_erase.cpp -o asan.out -fsanitize=address -static-liblsan -g

Compiling the above code with address sanitizer (asan) should flag the FMR as erase() for std::set is not used correctly here.

Executing asan.out gives the below output:

asan.out 0 2 4 6 8 10

[Enna1]

I'm not familiar with the implementation details of std::map in libstd++. IMHO, this is because the underlying memory is not actually freed when calling mp.erase(it), so asan can not detect it as a heap-use-after-free bug. The address sanitizer annotation for std::map is also not implemented in libstdc++, see https://gcc.gnu.org/legacy-ml/gcc-patches/2017-07/msg01314.html .

Under my test, this bug can be detected using clang with libcxx, see https://godbolt.org/z/xvzM7vYrq . @AdvenamTacet recently implemented more address sanitizer annotations for STL containers in libcxx.

Hi @vitalybuka @AdvenamTacet can you provide more information about address sanitizer annotations for STL containers in libcxx? Any plan to add asan annotation for associative containers like std::map ?

[AdvenamTacet]

Hey! I don't know much about libstdc++ std::set, but you are almost certainly right about memory not being actually freed.

Also, I don't think std::map/std::set is (ASan) annotated in any implementation.

can you provide more information about address sanitizer annotations for STL containers in libcxx?

In libc++17, my annotations were added to std::deque. std::basic_string annotations hopefully will be included in libc++18 (already implemented, waiting for dylib shipping commit).

ASan API (compiler-rt) was extended in LLVM16 to support more allocators during annotating containers and since libc++17 all allocators are annotated by default, improving state of the art of std::vector annotations and impacting std::deque annotations.

Any plan to add asan annotation for associative containers like std::map ?

I looked quickly (so, take my word with grain of salt) on libc++ implementation and it looks like some memory from the node is freed every time when element is removed from a __tree. Probably there is no benefit from std::set/std::map annotations in libc++. I don't know anything about libstdc++.

I don't think more containers than mentioned above will be annotated any time soon. Not sure if there is even a point to do it ever (if memory is freed "on erase", any use after free will be detected).

PS

Changes in libc++ have no effect on libstdc++ containers. We have working PoC for libstdc++ (12?) containers (deque and string), which we may make public, but those are more hacked implementations than something what can be simply merged.