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.

Sign up for GitHub

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

Jump to bottom

unpacker: add broadcom SAO image support #161

Merged
merged 1 commit into from
Oct 30, 2024
Merged

unpacker: add broadcom SAO image support #161

merged 1 commit into from
Oct 30, 2024

Conversation

andrewjlamarche
Copy link
Contributor

@andrewjlamarche andrewjlamarche commented Oct 24, 2024 •
edited
Loading

Add support for unpacking Broadcom SAO images, commonly found in DOCSIS cable modems.

NOTE: this requires a plaintext (e.g. not encrypted) file!

Depends:

@andrewjlamarche andrewjlamarche force-pushed the broadcom branch 3 times, most recently from 77097ff to 03ce96f Compare October 24, 2024 20:52
@andrewjlamarche andrewjlamarche marked this pull request as ready for review October 24, 2024 20:53
@jstucke
Copy link
Collaborator

jstucke commented Oct 25, 2024

Thank you for your contribution! I fear I must ask you to be patient, because @maringuu currently does not have time to finish his work on #128, so it could take a bit

@andrewjlamarche
Copy link
Contributor Author

I'll make the required changes in https://github.com/fkie-cad/fact_helper_file too, just didn't want to be redundant since it seems it'll be deprecated "soon".

@andrewjlamarche andrewjlamarche mentioned this pull request Oct 28, 2024
Merged
@andrewjlamarche
Copy link
Contributor Author

fkie-cad/fact_helper_file#36 added the changes to fact_helper_file as well. Ready for review without depending on @maringuu's changes

jstucke
jstucke reviewed Oct 29, 2024
View reviewed changes
Copy link
Collaborator

@jstucke jstucke left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Thank you for your contribution! Unpacking works fine with the test file you provided. I don't have any "real world" samples to test it with, though.
We recently switched to ruff as a linter and it seems to be unhappy about some parts: https://github.com/fkie-cad/fact_extractor/actions/runs/11507282602/job/32205501505?pr=161
Many of the issues can be automatically fixed with ruff check --fix [files...]

fact_extractor/plugins/unpacking/broadcom/test/test_plugin_broadcom_sao.py Outdated Show resolved Hide resolved
fact_extractor/plugins/unpacking/broadcom/test/test_plugin_broadcom_sao.py Outdated Show resolved Hide resolved
fact_extractor/plugins/unpacking/broadcom/test/test_plugin_broadcom_sao.py Outdated Show resolved Hide resolved
fact_extractor/plugins/unpacking/broadcom/code/broadcom-sao.py Outdated Show resolved Hide resolved
@andrewjlamarche
Copy link
Contributor Author

Thank you for your contribution! Unpacking works fine with the test file you provided. I don't have any "real world" samples to test it with, though. We recently switched to ruff as a linter and it seems to be unhappy about some parts: https://github.com/fkie-cad/fact_extractor/actions/runs/11507282602/job/32205501505?pr=161 Many of the issues can be automatically fixed with ruff check --fix [files...]

I've addressed these! I wasn't sure how to run the linter manually. Glad these things were caught.

@jstucke
Copy link
Collaborator

jstucke commented Oct 29, 2024

Thank you for your contribution! Unpacking works fine with the test file you provided. I don't have any "real world" samples to test it with, though. We recently switched to ruff as a linter and it seems to be unhappy about some parts: https://github.com/fkie-cad/fact_extractor/actions/runs/11507282602/job/32205501505?pr=161 Many of the issues can be automatically fixed with ruff check --fix [files...]

I've addressed these! I wasn't sure how to run the linter manually. Glad these things were caught.

It seems there are still some left (see https://github.com/fkie-cad/fact_extractor/actions/runs/11574200382/job/32228898616?pr=161)
The easiest way to reproduce and fix these is to install pre-commit (just run pip install pre-commit and then pre-commit install inside the checked out folder). Then all commit hooks are active and you can reproduce the result from the CI linter action. You can also run pre-commit run --files [files...] manually (this will also run ruff). Sorry for the inconvenience, I should have suggested that earlier.

@andrewjlamarche
Copy link
Contributor Author

Resolved the pathlib issue...

@andrewjlamarche
Copy link
Contributor Author

Okay, these should all be fixed, and I verified that the changes still work!

jstucke
jstucke approved these changes Oct 30, 2024
View reviewed changes
Copy link
Collaborator

@jstucke jstucke 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 to me. Thank you again for your contribution!

@jstucke jstucke merged commit bf65e87 into fkie-cad:master Oct 30, 2024
2 checks passed
@andrewjlamarche andrewjlamarche deleted the broadcom branch October 30, 2024 17:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jstucke jstucke jstucke approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@andrewjlamarche @jstucke