CPP: Add query for CWE-552 Files Accessible to External Parties when using rename

[ihsinme]

this query is looking for a mishandling situation rename. when a file is torn off from the program name and overwritten, this situation can lead to various actions, such as using links.

CVE-2012-0787

[jketema]

Hi @ihsinme,

Thanks for your contribution. A first few questions from my side.

[jketema]

Which of your test cases covers this?

[ihsinme]

test5 test6 test7 test8

[jketema]

I don't see this. If we look at test5 for example:

  if (rename(from,to)==0) // BAD
    return;
  f1 = fopen(from, "r");
  count = fread(data, 1, 1000, f1);
  fclose(f1);
  remove(to);
  f2 = fopen(to, "w");
  fwrite(data, count, 1, f2);
  fclose(f2);

Then, ifst.getThen().getASuccessor*() gives you all the statements and expressions textually below if (rename(from,to)==0), including fopen(from, "r") and fopen(to, "w"), so the forall does not hold.

[jketema]

Two general comments, which also apply to most of your other queries:

• I would help reviewers if you pick informative names for variables. For example, looking at this PR, from a reviewer's perspective it's hard to understand what variables like ec, ecd and ecv are supposed to represent. I'm not even sure if this is totally consistent throughout the query, we leads me to my other point.
• I would help if things that logically belong together in your queries occur textually close and to separate them from cases that are actually different. For example, in your current query, when I read this first part, I expect this following part to apply to both cases described in that first part. However, that is clearly not the case, as this rules out that ecd is a ComparisonOperator, while that is actually needed here.

[jketema]

To add to the above: this would allow us to give faster and better feedback, which is important, especially when there's a large number of PRs open.

[ihsinme]

You are absolutely right, I will try to follow the rules in the future.
Thanks.

[jketema]

Note by the way that cpp/world-writable-file-creation exists. Which should capture most, if not all cases here. That query just checks whether a file is created (using fopen, e.g.) using very lax permissions.

[ihsinme]

I apologize for the wasted time.

I am very serious about de-duplicates in my PRs, but in this case my review system failed and I didn't notice this request.

Sorry again, I'll try to be more careful in the future.