Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[clang][analyzer] Add checker 'security.SetgidSetuidOrder' #91445

Merged
merged 10 commits into from
May 22, 2024
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,6 @@ class SetgidSetuidOrderChecker : public Checker<check::PostCall, eval::Assume> {
void checkPostCall(const CallEvent &Call, CheckerContext &C) const;
ProgramStateRef evalAssume(ProgramStateRef State, SVal Cond,
bool Assumption) const;
const BugType *getBT() const { return &BT; }

private:
void processSetuid(ProgramStateRef State, const CallEvent &Call,
Expand Down Expand Up @@ -128,7 +127,7 @@ void SetgidSetuidOrderChecker::processSetuid(ProgramStateRef State,
State = State->set<LastSetuidCallSVal>(RetSym);
const NoteTag *Note = C.getNoteTag([this,
RetSym](PathSensitiveBugReport &BR) {
if (!BR.isInteresting(RetSym) || &BR.getBugType() != this->getBT())
if (!BR.isInteresting(RetSym) || &BR.getBugType() != &this->BT)
return "";
return "Call to 'setuid' found here that removes superuser privileges";
});
Expand Down
75 changes: 75 additions & 0 deletions clang/test/Analysis/setgid-setuid-order-notes.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,75 @@
// RUN: %clang_analyze_cc1 -analyzer-checker=core,security.SetgidSetuidOrder -analyzer-output=text -verify %s

typedef int uid_t;
typedef int gid_t;

int setuid(uid_t);
int setgid(gid_t);

uid_t getuid();
gid_t getgid();



void test_note_1() {
if (setuid(getuid()) == -1) // expected-note{{Assuming the condition is false}} \
// expected-note{{Taking false branch}}
return;
if (setuid(getuid()) == -1) // expected-note{{Call to 'setuid' found here that removes superuser privileges}} \
// expected-note{{Assuming the condition is false}} \
// expected-note{{Taking false branch}}
return;
if (setgid(getgid()) == -1) // expected-warning{{A 'setgid(getgid())' call following a 'setuid(getuid())' call is likely to fail}} \
// expected-note{{A 'setgid(getgid())' call following a 'setuid(getuid())' call is likely to fail}}
return;
}

void test_note_2() {
if (setuid(getuid()) == -1) // expected-note{{Call to 'setuid' found here that removes superuser privileges}} \
// expected-note{{Assuming the condition is false}} \
// expected-note{{Taking false branch}} \
// expected-note{{Assuming the condition is false}} \
// expected-note{{Taking false branch}}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// expected-note{{Assuming the condition is false}} \
// expected-note{{Taking false branch}} \
// expected-note{{Assuming the condition is false}} \
// expected-note{{Taking false branch}}
// expected-note 2 {{Assuming the condition is false}} \
// expected-note 2 {{Taking false branch}}

return;
if (setgid(getgid()) == -1) // expected-warning{{A 'setgid(getgid())' call following a 'setuid(getuid())' call is likely to fail}} \
// expected-note{{A 'setgid(getgid())' call following a 'setuid(getuid())' call is likely to fail}} \
// expected-note{{Assuming the condition is false}} \
// expected-note{{Taking false branch}}
return;
if (setuid(getuid()) == -1) // expected-note{{Call to 'setuid' found here that removes superuser privileges}} \
// expected-note{{Assuming the condition is false}} \
// expected-note{{Taking false branch}}
return;
if (setgid(getgid()) == -1) // expected-warning{{A 'setgid(getgid())' call following a 'setuid(getuid())' call is likely to fail}} \
// expected-note{{A 'setgid(getgid())' call following a 'setuid(getuid())' call is likely to fail}}
return;
}

int f_setuid() {
return setuid(getuid()); // expected-note{{Call to 'setuid' found here that removes superuser privileges}}
}

int f_setgid() {
return setgid(getgid()); // expected-warning{{A 'setgid(getgid())' call following a 'setuid(getuid())' call is likely to fail}} \
// expected-note{{A 'setgid(getgid())' call following a 'setuid(getuid())' call is likely to fail}}
}

void test_note_3() {
if (f_setuid() == -1) // expected-note{{Assuming the condition is false}} \
// expected-note{{Calling 'f_setuid'}} \
// expected-note{{Returning from 'f_setuid'}} \
// expected-note{{Taking false branch}}
return;
if (f_setgid() == -1) // expected-note{{Calling 'f_setgid'}}
return;
}

void test_note_4() {
if (setuid(getuid()) == 0) { // expected-note{{Assuming the condition is true}} \
// expected-note{{Call to 'setuid' found here that removes superuser privileges}} \
// expected-note{{Taking true branch}}
if (setgid(getgid()) == 0) { // expected-warning{{A 'setgid(getgid())' call following a 'setuid(getuid())' call is likely to fail}} \
// expected-note{{A 'setgid(getgid())' call following a 'setuid(getuid())' call is likely to fail}}
}
}
}
Loading