cfg: add library configuration for libselinux

[cgzones]

There are a couple false-positives and false-negatives:

• no warning of ignoredReturnValue for get_default_type():

  <function name="get_default_type">
    <returnValue type="int"/>
    <noreturn>false</noreturn>
    <use-retval/>
    <leak-ignore/>
    <arg nr="1" direction="in">
      <not-null/>
      <not-uninit/>
      <strz/>
    </arg>
    <arg nr="2" direction="out">
      <not-null/>
    </arg>
  </function>
  <memory>
    <alloc init="true" arg="2">get_default_type</alloc>
    <dealloc>free</dealloc>
  </memory>
  

  get_default_type("object_r", type2);  // does not report ignoredReturnValue
  

• wrong constVariablePointer report for selabel_open(), especially since the cleanup function selabel_close() does take a not pointer to non-const:

  <function name="selabel_open">
    <returnValue type="struct selabel_handle *"/>
    <noreturn>false</noreturn>
    <use-retval/>
    <leak-ignore/>
    <arg nr="1" direction="in">
      <not-uninit/>
      <not-bool/>
      <valid>0:5</valid>
    </arg>
    <arg nr="2" direction="in">
      <not-uninit/>
      <minsize type="argvalue" arg="3"/>
    </arg>
    <arg nr="3" direction="in">
      <not-uninit/>
      <not-bool/>
    </arg>
  </function>
  

  struct selabel_handle *hnd = selabel_open(SELABEL_CTX_FILE, NULL, 0);  // reports constVariablePointer
  

• missing memleak for getseuserbyname():

  <function name="getseuserbyname">
    <returnValue type="int"/>
    <noreturn>false</noreturn>
    <use-retval/>
    <leak-ignore/>
    <arg nr="1" direction="in">
      <not-null/>
      <not-uninit/>
      <strz/>
    </arg>
    <arg nr="2" direction="out">
      <not-null/>
    </arg>
    <arg nr="3" direction="out">
      <not-null/>
    </arg>
  </function>
  <memory>
    <alloc init="true" arg="2">getseuserbyname</alloc>
    <dealloc>free</dealloc>
  </memory>
  <memory>
    <alloc init="true" arg="3">getseuserbyname</alloc>
    <dealloc>free</dealloc>
  </memory>
  

  void getseuserbyname_fail2(void)
  {
    char *seuser, *level;
    getseuserbyname("root", &seuser, &level);
    free(level);
  
    // seuser is leaked; no memleak report
  }
  

[firewave]

Thanks for your contribution.

You also need to install the library in the CI job in CI-unixish.yml which runs the test in strict mode.

Also the configuration seems to have an unclosed comment.

[cgzones]

You also need to install the library in the CI job in CI-unixish.yml which runs the test in strict mode.

Done.

Also the configuration seems to have an unclosed comment.

True, fixed.

Any ideas for the above issues?

[chrchr-github]

struct selabel_handle *hnd = selabel_open(SELABEL_CTX_FILE, NULL, 0);  // reports constVariablePointer

This should depend on how hnd is used later on. It's not clear if this is a false positive yet. It looks like a true positive in void selabel_fail1(void),

[chrchr-github]

get_default_type("object_r", type2);  // does not report ignoredReturnValue

It is reported if the <memory> configuration is removed. So I suppose we might suppress the ignoredReturnValue in favor of memleak (which should only happen for functions which return the allocated pointer). Could be fixed by adding

const bool warn = [&]() {
    if (tok->function() && (tok->function()->isAttributeNodiscard() || tok->function()->isAttributePure() || tok->function()->isAttributeConst()))
        return true;
    // avoid duplicate warnings for resource-allocating functions
    if (retvalTy == Library::UseRetValType::DEFAULT) {
        if (const Library::AllocFunc* info = mSettings->library.getAllocFuncInfo(tok))
            return info->arg > 0;
    }
    return false;
    }();

in checkIgnoredReturnValue(). Edit: memleak works as expected. Outparam allocations are supported, but maybe not always handled correctly. See e.g. https://github.com/danmar/cppcheck/commit/63a5a71c20791ec74380499bda4040bb9faf77d1

[chrchr-github]

void getseuserbyname_fail2(void)
{
  char *seuser, *level;
  getseuserbyname("root", &seuser, &level);
  free(level);

  // seuser is leaked; no memleak report
}

Looks like Library::AllocFunc supports only one arg.

[chrchr-github]

struct selabel_handle *hnd = selabel_open(SELABEL_CTX_FILE, NULL, 0);  // reports constVariablePointer

This should depend on how hnd is used later on. It's not clear if this is a false positive yet. It looks like a true positive in void selabel_fail1(void),

The FP involving selabel_lookup() can be fixed by removing the argument direction for hnd. https://github.com/danmar/cppcheck/pull/6451 might improve that situation.

[cgzones]

struct selabel_handle *hnd = selabel_open(SELABEL_CTX_FILE, NULL, 0);  // reports constVariablePointer

This should depend on how hnd is used later on. It's not clear if this is a false positive yet. It looks like a true positive in void selabel_fail1(void),

The FP involving selabel_lookup() can be fixed by removing the argument direction for hnd. #6451 might improve that situation.

Thanks; changed the argument direction in selabel_lookup() to inout

[chrchr-github]

Maybe we should add include detection here: https://github.com/danmar/cppcheck/blob/dc385f3b4a4b94507ff21f2ec428fd6cb0366ea5/tools/donate_cpu_lib.py#L680

[danmar]

Maybe we should add include detection here:

https://github.com/danmar/cppcheck/blob/dc385f3b4a4b94507ff21f2ec428fd6cb0366ea5/tools/donate_cpu_lib.py#L680

sorry is this related to this PR?

[chrchr-github]

Maybe we should add include detection here: https://github.com/danmar/cppcheck/blob/dc385f3b4a4b94507ff21f2ec428fd6cb0366ea5/tools/donate_cpu_lib.py#L680

sorry is this related to this PR?

If we could detect the corresponding headers, we would automatically load the new library in daca (as far as I understand it). Same with the emscripten PR https://github.com/danmar/cppcheck/pull/6503

[firewave]

If we could detect the corresponding headers, we would automatically load the new library in daca (as far as I understand it). Same with the emscripten PR #6503

Those need to be disabled though if this library did not existing in the previous stable version or we will get a failure.

Also please remember to bump the version of the script when changing it.

[chrchr-github]

Those need to be disabled though if this library did not existing in the previous stable version or we will get a failure.

So we should add it, but leave it commented out until 2.15 is released?

[firewave]

So we should add it, but leave it commented out until 2.15 is released?

Yes.

[chrchr-github]

There is a branch conflict now.

[firewave]

This is also still missing the library detection in daca.

[firewave]

Thanks. Please update CLIENT_VERSION in tools/donate_cpu_lib.py as well.

[chrchr-github]

Thanks. Please update CLIENT_VERSION in tools/donate_cpu_lib.py as well.

AFAIK we intended to leave the detection commented out until 2.15 is released. Not sure if the version number needs to be bumped if there is no functional change.

[firewave]

AFAIK we intended to leave the detection commented out until 2.15 is released.

Good catch,

Not sure if the version number needs to be bumped if there is no functional change

For this file I would prefer it to get an indication on how old the client actually is.

[chrchr-github]

@cgzones How do you wish to be credited in the Authors file?

[cgzones]

How do you wish to be credited in the Authors file?

As 'Christian Göttsche'.

Thanks for your support and this amazing tool in general.

[chrchr-github]

How do you wish to be credited in the Authors file?

As 'Christian Göttsche'.

Done: https://github.com/danmar/cppcheck/commit/15bdd976b42ba5d693703d5baf6c5f1673182850