NCAR / micm

A model-independent chemistry module for atmosphere models
https://ncar.github.io/micm/
Apache License 2.0
6 stars 4 forks source link

Clang-tidy cert-* additions #520

Closed mwaxmonsky closed 2 months ago

mwaxmonsky commented 2 months ago

Is your feature request related to a problem? Please describe. No

Describe the solution you'd like Modify .clang-tidy to flag cert-* checks: cert-*,

Describe alternatives you've considered N\A

Additional context cert-dcl50-cpp ON

A variadic function using a C-style ellipsis (hereafter called a C-style variadic function) has no mechanisms to check the type safety of arguments being passed to the function or to check that the number of arguments being passed matches the semantics of the function definition. Consequently, a runtime call to a C-style variadic function that passes inappropriate arguments yields undefined behavior. Such undefined behavior could be exploited to run arbitrary code.

Prevents

#include <cstdarg>

int add(int first, int second, ...) {
  int r = first + second;  
  va_list va;
  va_start(va, second);
  while (int v = va_arg(va, int)) {
    r += v;
  }
  va_end(va);
  return r;
}

Recommends

#include <type_traits>

template <typename Arg, typename... Ts, typename std::enable_if<std::is_integral<Arg>::value>::type * = nullptr>
int add(Arg i, Arg j, Ts... all) {
  int values[] = { j, all... };
  int r = i;
  for (auto v : values) {
    r += v;
  }
  return r;
}

cert-dcl58-cpp ON

Modification of the std or posix namespace can result in undefined behavior.

namespace std {
  int x; // warning: modification of 'std' namespace can result in undefined behavior [cert-dcl58-cpp]
}

namespace posix::a { // warning: modification of 'posix' namespace can result in undefined behavior
}

template <>
struct ::std::hash<long> { // warning: modification of 'std' namespace can result in undefined behavior
  unsigned long operator()(const long &K) const {
    return K;
  }
};

struct MyData { long data; };

template <>
struct ::std::hash<MyData> { // no warning: specialization with user-defined type
  unsigned long operator()(const MyData &K) const {
    return K.data;
  }
};

namespace std {
  template <>
  void swap<bool>(bool &a, bool &b); // warning: modification of 'std' namespace can result in undefined behavior

  template <>
  bool less<void>::operator()<MyData &&, MyData &&>(MyData &&, MyData &&) const { // warning: modification of 'std' namespace can result in undefined behavior
    return true;
  }
}

cert-env33-c ON

This check flags calls to system(), popen(), and _popen(), which execute a command processor. Other POSIX compliant commands should exist to do the minimum needed work reducing the risk of being abused.

cert-err33-c ON

Warns on unused function return values. Assuming that all calls to such functions will succeed and failing to check the return value for an indication of an error is a dangerous practice that may lead to unexpected or undefined behavior when an error occurs. It is essential that programs detect and appropriately handle all errors in accordance with an error-handling policy.

Alias of check bugprone-unused-return-value with a fixed set of C-style functions.

cert-err34-c ON

This check flags calls to string-to-number conversion functions that do not verify the validity of the conversion, such as atoi() or scanf().

cert-err52-cpp ON

This check flags all call expressions involving setjmp() and longjmp(). These facilities bypass automatic resource management and can result in undefined behavior, commonly including resource leaks and denial-of-service attacks

cert-err58-cpp ON

This check flags all static or thread_local variable declarations where the initializer for the object may throw an exception.

struct S {
  S() noexcept(false);
};

static S globalS;

In this noncompliant example, the constructor for S may throw an exception that is not caught when globalS is constructed during program startup.

cert-err60-cpp ON

In this noncompliant code example, an exception of type S is thrown in f(). However, because S has a std::string data member, and the copy constructor for std::string is not declared noexcept, the implicitly-defined copy constructor for S is also not declared to be noexcept. In low-memory situations, the copy constructor for std::string may be unable to allocate sufficient memory to complete the copy operation, resulting in a std::bad_alloc exception being thrown.

#include <exception>
#include <string>

class S : public std::exception {
  std::string m;
public:
  S(const char *msg) : m(msg) {}

  const char *what() const noexcept override {
    return m.c_str();  
  }
};

void g() {
  // If some condition doesn't hold...
  throw S("Condition did not hold");
}

void f() {
  try {
    g();
  } catch (S &s) {
    // Handle error
  }
}

This compliant solution assumes that the type of the exception object can inherit from std::runtime_error, or that type can be used directly. Unlike std::string, a std::runtime_error object is required to correctly handle an arbitrary-length error message that is exception safe and guarantees the copy constructor will not throw

#include <stdexcept>
#include <type_traits>

struct S : std::runtime_error {
  S(const char *msg) : std::runtime_error(msg) {}
};

static_assert(std::is_nothrow_copy_constructible<S>::value,
              "S must be nothrow copy constructible");

void g() {
  // If some condition doesn't hold...
  throw S("Condition did not hold");
}

void f() {
  try {
    g();
  } catch (S &s) {
    // Handle error
  }
}

cert-flp30-c ON

This check flags for loops where the induction expression has a floating-point type. Different implementations have different precision limitations, and to keep code portable, floating-point variables must not be used as the loop induction variable.

void func(void) {
  for (float x = 0.1f; x <= 1.0f; x += 0.1f) {
    /* Loop may iterate 9 or 10 times */
  }
}

cert-mem57-cpp ON

This check flags uses of default operator new where the type has extended alignment (an alignment greater than the fundamental alignment). Using improperly aligned pointers results in undefined behavior, typically leading to abnormal termination.

In the following noncompliant code example, the new expression is used to invoke the default operator new to obtain storage in which to then construct an object of the user-defined type Vector with alignment that exceeds the fundamental alignment of most implementations (typically 16 bytes).

struct alignas(32) Vector {
  char elems[32];
};

Vector *f() {
  Vector *pv = new Vector;
  return pv;
}

cert-msc50-cpp ON

This check warns for the usage of std::rand() as the result can be predictable.

cert-msc51-cpp MAYBE - Needed for reproducability

This check flags all pseudo-random number engines, engine adaptor instantiations and srand() when initialized or seeded with default argument, constant expression or any user-configurable type.

void foo() {
  std::mt19937 engine1; // Diagnose, always generate the same sequence
  std::mt19937 engine2(1); // Diagnose
  engine1.seed(); // Diagnose
  engine2.seed(1); // Diagnose

  std::time_t t;
  engine1.seed(std::time(&t)); // Diagnose, system time might be controlled by user

  int x = atoi(argv[1]);
  std::mt19937 engine3(x);  // Will not warn
}

cert-oop57-cpp ON

Flags use of the C standard library functions memset, memcpy and memcmp and similar derivatives on non-trivial types.

Most violations of this rule will result in abnormal program behavior. However, overwriting implementation details of the object representation can lead to code execution vulnerabilities.

cert-oop58-cpp ON

Finds assignments to the copied object and its direct or indirect members in copy constructors and copy assignment operators.

Copy operations that mutate the source operand or global state can lead to unexpected program behavior. Using such a type in a Standard Template Library container or algorithm can also lead to undefined behavior.