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

Add refcounted-usage rule #714

Merged
merged 2 commits into from
Nov 21, 2024
Merged

Add refcounted-usage rule #714

merged 2 commits into from
Nov 21, 2024

Conversation

stoletheminerals
Copy link
Contributor

Adds a rule that catches base::RefCounted* usage. Also this PR modifies other C++ rules to include .mm files

};

// ruleid: refcounted-usage
base::RefCountedData<int> shared_integer(42);

Choose a reason for hiding this comment

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

reported by reviewdog 🐶
[semgrep] Reference counting is occasionally useful but is more often a sign that someone isn't thinking carefully about ownership. Use it when ownership is truly shared (for example, multiple tabs sharing the same renderer process), not for when lifetime management is difficult to reason about.

Source: https://github.com/brave/security-action/blob/main/assets/semgrep_rules/client/refcounted-usage.yaml


Cc @stoletheminerals @thypon @cdesouza-chromium @bridiver

};

// ruleid: refcounted-usage
using MyRefCountedType = base::RefCounted<SomeType>;

Choose a reason for hiding this comment

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

reported by reviewdog 🐶
[semgrep] Reference counting is occasionally useful but is more often a sign that someone isn't thinking carefully about ownership. Use it when ownership is truly shared (for example, multiple tabs sharing the same renderer process), not for when lifetime management is difficult to reason about.

Source: https://github.com/brave/security-action/blob/main/assets/semgrep_rules/client/refcounted-usage.yaml


Cc @stoletheminerals @thypon @cdesouza-chromium @bridiver

@@ -0,0 +1,26 @@
// ruleid: refcounted-usage
class MyClass : public base::RefCounted<MyClass> {

Choose a reason for hiding this comment

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

reported by reviewdog 🐶
[semgrep] Reference counting is occasionally useful but is more often a sign that someone isn't thinking carefully about ownership. Use it when ownership is truly shared (for example, multiple tabs sharing the same renderer process), not for when lifetime management is difficult to reason about.

Source: https://github.com/brave/security-action/blob/main/assets/semgrep_rules/client/refcounted-usage.yaml


Cc @stoletheminerals @thypon @cdesouza-chromium @bridiver

std::shared_ptr<MyClass> better_alternative;

// ruleid: refcounted-usage
class NestedRefCounted : public base::RefCountedThreadSafe<NestedRefCounted> {

Choose a reason for hiding this comment

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

reported by reviewdog 🐶
[semgrep] Reference counting is occasionally useful but is more often a sign that someone isn't thinking carefully about ownership. Use it when ownership is truly shared (for example, multiple tabs sharing the same renderer process), not for when lifetime management is difficult to reason about.

Source: https://github.com/brave/security-action/blob/main/assets/semgrep_rules/client/refcounted-usage.yaml


Cc @stoletheminerals @thypon @cdesouza-chromium @bridiver

// ruleid: refcounted-usage
class NestedRefCounted : public base::RefCountedThreadSafe<NestedRefCounted> {
// ruleid: refcounted-usage
base::RefCountedData<std::string> nested_data_;

Choose a reason for hiding this comment

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

reported by reviewdog 🐶
[semgrep] Reference counting is occasionally useful but is more often a sign that someone isn't thinking carefully about ownership. Use it when ownership is truly shared (for example, multiple tabs sharing the same renderer process), not for when lifetime management is difficult to reason about.

Source: https://github.com/brave/security-action/blob/main/assets/semgrep_rules/client/refcounted-usage.yaml


Cc @stoletheminerals @thypon @cdesouza-chromium @bridiver

};

// ruleid: refcounted-usage
class ThreadSafeClass : public base::RefCountedThreadSafe<ThreadSafeClass> {

Choose a reason for hiding this comment

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

reported by reviewdog 🐶
[semgrep] Reference counting is occasionally useful but is more often a sign that someone isn't thinking carefully about ownership. Use it when ownership is truly shared (for example, multiple tabs sharing the same renderer process), not for when lifetime management is difficult to reason about.

Source: https://github.com/brave/security-action/blob/main/assets/semgrep_rules/client/refcounted-usage.yaml


Cc @stoletheminerals @thypon @cdesouza-chromium @bridiver

using MyRefCountedType = base::RefCounted<SomeType>;

// ruleid: refcounted-usage
class NestedRefCounted : public base::RefCountedThreadSafe<NestedRefCounted> {

Choose a reason for hiding this comment

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

reported by reviewdog 🐶
[semgrep] Reference counting is occasionally useful but is more often a sign that someone isn't thinking carefully about ownership. Use it when ownership is truly shared (for example, multiple tabs sharing the same renderer process), not for when lifetime management is difficult to reason about.

Source: https://github.com/brave/security-action/blob/main/assets/semgrep_rules/client/refcounted-usage.yaml


Cc @stoletheminerals @thypon @cdesouza-chromium @bridiver

// ruleid: refcounted-usage
class NestedRefCounted : public base::RefCountedThreadSafe<NestedRefCounted> {
// ruleid: refcounted-usage
base::RefCountedData<std::string> nested_data_;

Choose a reason for hiding this comment

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

reported by reviewdog 🐶
[semgrep] Reference counting is occasionally useful but is more often a sign that someone isn't thinking carefully about ownership. Use it when ownership is truly shared (for example, multiple tabs sharing the same renderer process), not for when lifetime management is difficult to reason about.

Source: https://github.com/brave/security-action/blob/main/assets/semgrep_rules/client/refcounted-usage.yaml


Cc @stoletheminerals @thypon @cdesouza-chromium @bridiver

Copy link

@cdesouza-chromium cdesouza-chromium left a comment

Choose a reason for hiding this comment

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

LGTM

@stoletheminerals stoletheminerals merged commit 19fa03f into main Nov 21, 2024
7 checks passed
@stoletheminerals stoletheminerals deleted the refcounted-usage branch November 21, 2024 18:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants