Skip to content

Fix CRX3 ZIP payload extraction - #951

Merged
babenek merged 6 commits into
Samsung:mainfrom
arunimshukla:fix/crx3-payload-offset
Sep 21, 2026
Merged

Fix CRX3 ZIP payload extraction#951
babenek merged 6 commits into
Samsung:mainfrom
arunimshukla:fix/crx3-payload-offset

Conversation

@arunimshukla

@arunimshukla arunimshukla commented Sep 20, 2026 •
edited by babenek
Loading

Copy link
Copy Markdown
Contributor

Description

  • Parse CRX2 and CRX3 headers using their version-specific layouts.
  • Reject unsupported versions and truncated headers instead of returning a misaligned payload.
  • Add regression coverage for valid CRX2 and CRX3 payloads plus malformed inputs.

Before this change, CrxScanner.match accepted CRX3 files, but zip_extract always interpreted bytes 8–15 as the CRX2 public-key and signature lengths. For CRX3, bytes 8–11 are one protobuf-header length followed by the header, so the scanner could calculate the wrong ZIP offset and miss credentials inside modern Chrome extensions.

Chromium format reference: https://chromium.googlesource.com/chromium/src/+/main/components/crx_file/crx3.proto

How has this been tested?

  • UnitTest
  • Benchmark
python -m pytest tests/deep_scanner/test_crx_scanner.py -q
# 4 passed

python -m pytest tests/deep_scanner -q
# 96 passed

yapf --style .style.yapf --diff credsweeper/deep_scanner/crx_scanner.py tests/deep_scanner/test_crx_scanner.py
flake8 credsweeper/deep_scanner/crx_scanner.py tests/deep_scanner/test_crx_scanner.py
pylint --py-version=3.12 credsweeper/deep_scanner/crx_scanner.py
# 10.00/10

mypy --config-file .mypy.ini --python-version=3.12 credsweeper
# Success: no issues found in 175 source files

Benchmark was not run because this changes header parsing only and does not alter detection rules or model scoring.

@arunimshukla
arunimshukla requested a review from a team as a code owner September 20, 2026 17:35
@babenek

babenek commented Sep 20, 2026

Copy link
Copy Markdown
Contributor

Hi @arunimshukla,
thanks you for the fix! It really works with various CRX files and the code is ok.
Unfortunately, not all action tests are passed. I prepared the fix arunimshukla#1 - apply it in web application to make a last signed commit in the PR or merge it with fast-forward.
Then the actions may be started.
Best Regards

@arunimshukla

arunimshukla commented Sep 21, 2026

Copy link
Copy Markdown
Contributor Author

Hi @babenek, thanks for preparing the fix and for testing it against different CRX files.

I've merged your updates from arunimshukla#1 into this branch (2cf200c), so it's now in sync with the latest main. It's ready for the Actions to run whenever you are. If anything else fails, let me know and I'll address it.

babenek
babenek previously approved these changes Sep 21, 2026
View reviewed changes

@babenek babenek left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thank you for the fix!

@codecov-commenter

codecov-commenter commented Sep 21, 2026 •
edited
Loading

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.49%. Comparing base (1732bd5) to head (0b16b84).

Additional details and impacted files 
@@            Coverage Diff             @@
##             main     #951      +/-   ##
==========================================
+ Coverage   90.46%   90.49%   +0.02%     
==========================================
  Files         166      166              
  Lines        7164     7172       +8     
  Branches     1205     1208       +3     
==========================================
+ Hits         6481     6490       +9     
  Misses        456      456              
+ Partials      227      226       -1

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

babenek
babenek reviewed Sep 21, 2026
View reviewed changes
Comment thread .github/workflows/check.yml Resolved
babenek
babenek reviewed Sep 21, 2026
View reviewed changes
Comment thread credsweeper/deep_scanner/crx_scanner.py Resolved
Pomidorry
Pomidorry approved these changes Sep 21, 2026
View reviewed changes

@Pomidorry Pomidorry left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

@babenek
babenek merged commit 7e54102 into Samsung:main Sep 21, 2026
32 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@xDizzix xDizzix xDizzix approved these changes
@babenek babenek babenek approved these changes
@Pomidorry Pomidorry Pomidorry approved these changes
@Yullia Yullia Awaiting requested review from Yullia
@iuriimet iuriimet Awaiting requested review from iuriimet
@meanrin meanrin Awaiting requested review from meanrin
@csh519 csh519 Awaiting requested review from csh519
@kmnls kmnls Awaiting requested review from kmnls

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@arunimshukla @babenek @codecov-commenter @xDizzix @Pomidorry