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

Improve browser stealer & add SQLite lib detection #757

Merged
merged 7 commits into from
Aug 19, 2023

Conversation

Still34
Copy link
Contributor

@Still34 Still34 commented May 11, 2023

Summary

This PR adds detection for SQL statements related to cookie accesses, fixes one of the regexes, as well as adding rudimentary support for detection static sqlite3/cppsqlite3 lib linking. The relevant samples will be submitted to the test repo once I get the go ahead from the maintainers.

Copy link
Collaborator

@williballenthin williballenthin left a comment

Choose a reason for hiding this comment

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

logic looks great, excited for the referenced file so we can merge this

thank you!

Still34 added a commit to Still34/capa-testfiles that referenced this pull request May 12, 2023
Copy link
Collaborator

@mr-tz mr-tz left a comment

Choose a reason for hiding this comment

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

Thanks! I've added comments to make the linter happy.

@mr-tz
Copy link
Collaborator

mr-tz commented May 15, 2023

from the linter:

  FAIL: rule string quotes incorrect: add double quotes to "/\\+(Edge|Chrome|Chromium|Brave\-Browser|YandexBrowser|Kometa|Orbitum|Dragon|Torch|Amigo)\\+User Data\\+Default(\\+Network)?\\+(Cookies|Login Data)/i"

@Still34
Copy link
Contributor Author

Still34 commented May 23, 2023

Ah right, oops, accidentally used substring instead of string.

@mr-tz
Copy link
Collaborator

mr-tz commented May 26, 2023

Now, we get this from the linter:
FAIL: doesn't match on referenced example: Fix the rule logic or provide a different example

@Still34
Copy link
Contributor Author

Still34 commented Jun 30, 2023

Sorry for the stale PR - it looks like the linter passes the test locally for me though.

 capa-rules  py -3 "E:\repos\capa\scripts\lint.py" --thorough  --samples E:\repos\capa\tests\data -t "gather chrome based browser login information" .
INFO:lint:successfully loaded 799 rules
INFO:lint:collecting potentially referenced samples
linting rule: gather chrome based browser log...: 100%|██████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████| 1/1 [00:00<00:00,  2.96rule/s]
INFO:lint:no lints failed, nice!

@mr-tz
Copy link
Collaborator

mr-tz commented Jul 6, 2023

It doesn't match for me. The function doesn't contain the SELECT strings. Can you double check what's off?

- Fix erroneous regex capture
+ Add detections for cookies gathering
+ Add generic browser detection (some webkit browser for some reason uses the same chromium-based paths?)

Signed-off-by: Still Hsu <[email protected]>
- Typically used along with browser data collection

Signed-off-by: Still Hsu <[email protected]>
Signed-off-by: Still Hsu <[email protected]>
@Still34 Still34 force-pushed the patches/browser-stealer-improve branch from da4afb4 to 4520523 Compare August 8, 2023 14:00
@Still34
Copy link
Contributor Author

Still34 commented Aug 8, 2023

After having a headache trying to figure out what went wrong for 30 minutes, finally figured out what was causing it to not match - it was due to the default scope the previous rule used, switching to file fixed it. It should pass the linter now hopefully.

@Still34 Still34 requested a review from mr-tz August 8, 2023 14:10
Copy link
Collaborator

@mr-tz mr-tz left a comment

Choose a reason for hiding this comment

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

looks good, thank you!

@mr-tz mr-tz merged commit 037ca83 into mandiant:master Aug 19, 2023
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants