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 Screenshot Detection by Modifying capture-screenshot.yml #1007

Open
wants to merge 8 commits into
base: master
Choose a base branch
from

Conversation

akh7177
Copy link

@akh7177 akh7177 commented Feb 24, 2025

Added GDI, GDIP, and DirectX API calls to improve detection accuracy for various screenshot capture methods.

This detects screenshot capture that uses GDI+ routines as well as CreateDC with other arguments along with DISPLAY. The test files include two executables that demonstrate the improved detection.

closes #981

akh7177 and others added 2 commits February 24, 2025 22:08
Improve Screenshot Detection by Modifying capture-screenshot.yml
Comment on lines 51 to 68
- and:
- or:
- api: "GetDC"
- and:
- api: "CreateDC"
- optional:
- string: "DISPLAY"
- or:
- and:
- api: "SelectObject"
- api: "DrawText"
- and:
- api: "CreateCompatibleDC"
- api: "CreateCompatibleBitmap"
- or:
- api: "BitBlt"
- api: "StretchBlt"
- api: "PrintWindow"
Copy link
Collaborator

Choose a reason for hiding this comment

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

some of this seems to be duplicated from above, right? would you please merge the logic where reasonable?

Copy link
Author

Choose a reason for hiding this comment

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

Hello @williballenthin ,

I've tried to remove the duplicated parts of the code and committed the changes. Please let me know if further improvements are required.

@akh7177 akh7177 force-pushed the add-screenshot-rule branch from adb3858 to 4cd4879 Compare February 25, 2025 19:19
@akh7177 akh7177 force-pushed the add-screenshot-rule branch from 9fe63a9 to ea56ada Compare February 26, 2025 17:19
@mike-hunhoff
Copy link
Collaborator

@akh7177 lints are failing now:

 capture screenshot
  FAIL: rule contains one or more statements with a single child statement: 
remove the superfluous parent statement: 
and(or(api(BitBlt),api(StretchBlt),api(PrintWindow)))
  FAIL: rule format incorrect: use scripts/capafmt.py or adjust as follows

You can run lints locally by installing capa for development using the instructions found here.

@akh7177
Copy link
Author

akh7177 commented Mar 11, 2025

@mike-hunhoff I modified the file and made sure that it passes lint.py given that my new test binary is added .
Could you please check it?

Copy link
Collaborator

@mike-hunhoff mike-hunhoff left a comment

Choose a reason for hiding this comment

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

Good work @akh7177 , I've left comments for your review.

Copy link
Collaborator

@mike-hunhoff mike-hunhoff left a comment

Choose a reason for hiding this comment

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

Thanks @akh7177 , I noticed a few things and left comments for you to address.

@@ -43,3 +47,31 @@ rule:
- and:
- api: BitBlt
- api: System.Drawing.Graphics::CopyFromScreen
- and:
- api: "SelectObject"
Copy link
Collaborator

Choose a reason for hiding this comment

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

api features do not require ". Please remove the " from this api feature and all that follow.

Copy link
Author

Choose a reason for hiding this comment

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

Sure. I'll remove the quotes!

Comment on lines +63 to +64
- api: "Gdiplus::Graphics::FromHDC"
- api: "Gdiplus::Graphics::CopyFromScreen"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should these be api or function-name features?

Copy link
Author

Choose a reason for hiding this comment

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

Hello @mike-hunhoff !
According to my understanding, Gdiplus is not from a statically linked library right?

Comment on lines +75 to +76
- api: "Direct3DCreate9"
- api: "IDirect3DDevice9::GetFrontBufferData"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should these be api or function-name features?

Copy link
Author

Choose a reason for hiding this comment

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

Over here too, Direct3DCreate 9 is a direct import of a dll file right?

Copy link
Author

Choose a reason for hiding this comment

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

Furthermore, changing these features to function-name would require me to change the static scope of the rule to file. Would it not increase the number of false positives?

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.

false negative for screenshot
3 participants