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

Fix issue Network Services / Network Hosts #3175

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

Conversation

Phasip
Copy link

@Phasip Phasip commented Oct 23, 2024

Pull Request

Ref: #3174

Category

Bugfix

Feature/Issue Description

  1. type field was renamed to ntype many years ago, fix modules to match this.
  2. the hooked_browser_id field is a reference to the hooked_browser tables id field, we cannot use session_id for this, instead we need to use the session_id to look up which hooked_browser_id to use.
  3. The javascript part of the UI still attempted to use the type field, change to use the ntype field.

Test Cases

None.

@Phasip Phasip had a problem deploying to Integrate Pull Request October 23, 2024 19:07 — with GitHub Actions Failure
@stephenakq
Copy link
Collaborator

Thank you for your contribution! Could you please provide more details about the changes in this PR?

@Phasip
Copy link
Author

Phasip commented Oct 26, 2024

Of course! Sorry for the big blob commit.
The issue we are fixing is that the UI tables for network services and network hosts is broken and does not show anything even when services/hosts are detected.

So, most files changed are inserts into network_services and network_hosts tables. (files: browserdetails.rb and multiple module.rb). These have the same type of change:

session_id = @datastore['beefhook']
...
BeEF::Core::Models::NetworkService.create(hooked_browser_id: session_id, ..., type: type)

has become

session_id = @datastore['beefhook']
hooked_browser = BeEF::Core::Models::HookedBrowser.where(session: session_id).first
BeEF::Core::Models::NetworkService.create(hooked_browser: hooked_browser, ..., ntype: type)

This change is because the hooked_browser_id should be an integer reference to the hooked_browser and not the session_id string in @datastore['beefhook']. The previous version resulted in network_services.hooked_browser_id always being 0.

Then we have the change to ZombieTabNetwork.js
which simply fetches the ntype field instead of type from network_services, this is because type does not exist as it was renamed a long time ago. Without this change the type field of the network services tab in the ui is always empty.

Finally the change to network.rb
The UI makes request towards /services/:id where :id is the session_id of the hooked browser, however previously the code attempted to use this string for the integer network_services.hooked_browser_id field. This has been changed.
Additionally the /hosts/:id already did this, however the code used .distinct which didn't seem to work when I implemented the change for /services/:id so I changed both to .first.
As session_id should be unique for the hooked browser db table there is no issue using .first without checking that there is only one result - this is also how it is done in multiple other places in the code.
In practice another change should be made to make session_id unique in the db to ensure this is true.

@zinduolis zinduolis self-requested a review November 6, 2024 05:25
@zinduolis
Copy link
Collaborator

Hi @Phasip, thanks for your contribution and thorough description. I'm reviewing your PR.

Meanwhile, could you please rebase it with the latest master? I have recently fixed up the automated Browserstack testing and done some dependency upgrades. Once you rebase your PR, this should run and tell us if any issues.

Thanks

@Phasip Phasip had a problem deploying to Integrate Pull Request November 7, 2024 05:41 — with GitHub Actions Failure
@zinduolis
Copy link
Collaborator

Thanks, @Phasip , for rebasing, I'm running the tests now. Please advise what testing you have done that no integrity within the framework has been affected. If you are not sure about what testing should be run, you could try running bundle exec rake, also starting the framework and then executing debug modules and making sure no errors are thrown.

@Phasip Phasip temporarily deployed to Integrate Pull Request November 7, 2024 08:34 — with GitHub Actions Inactive
@Phasip
Copy link
Author

Phasip commented Nov 9, 2024

Thanks, @Phasip , for rebasing, I'm running the tests now. Please advise what testing you have done that no integrity within the framework has been affected. If you are not sure about what testing should be run, you could try running bundle exec rake, also starting the framework and then executing debug modules and making sure no errors are thrown.

The testing I have done is to verify that the previous exceptions are not thrown and that services appear in the services tab, which they did not before.

I ran bundle exec rake now and received the same errors as on the master branch.

Failures:

  1) BeEF Modules loaded successfully
     Failure/Error: expect(BeEF::Module.is_loaded(k)).to be(true)
     
       expected true
            got false
     # ./spec/beef/core/modules_spec.rb:19:in `block (3 levels) in <top (required)>'
     # ./spec/beef/core/modules_spec.rb:13:in `each'
     # ./spec/beef/core/modules_spec.rb:13:in `block (2 levels) in <top (required)>'
     # ./spec/spec_helper.rb:82:in `block (3 levels) in <top (required)>'
     # /usr/local/bundle/gems/activerecord-7.2.2/lib/active_record/connection_adapters/abstract/transaction.rb:616:in `block in within_new_transaction'
     # /usr/local/bundle/gems/activesupport-7.2.2/lib/active_support/concurrency/null_lock.rb:9:in `synchronize'
     # /usr/local/bundle/gems/activerecord-7.2.2/lib/active_record/connection_adapters/abstract/transaction.rb:613:in `within_new_transaction'
     # /usr/local/bundle/gems/activerecord-7.2.2/lib/active_record/connection_adapters/abstract/database_statements.rb:361:in `transaction'
     # /usr/local/bundle/gems/activerecord-7.2.2/lib/active_record/transactions.rb:234:in `block in transaction'
     # /usr/local/bundle/gems/activerecord-7.2.2/lib/active_record/connection_adapters/abstract/connection_pool.rb:415:in `with_connection'
     # /usr/local/bundle/gems/activerecord-7.2.2/lib/active_record/connection_handling.rb:296:in `with_connection'
     # /usr/local/bundle/gems/activerecord-7.2.2/lib/active_record/transactions.rb:233:in `transaction'
     # ./spec/spec_helper.rb:80:in `block (2 levels) in <top (required)>'

Finished in 2.19 seconds (files took 1.04 seconds to load)
131 examples, 1 failure, 3 pending

Failed examples:

rspec ./spec/beef/core/modules_spec.rb:3 # BeEF Modules loaded successfully

Also, I do not think the BrowserStack error is due to my changes.

@Phasip Phasip had a problem deploying to Integrate Pull Request November 9, 2024 06:37 — with GitHub Actions Failure
@Phasip Phasip had a problem deploying to Integrate Pull Request November 10, 2024 10:48 — with GitHub Actions Failure
@zinduolis
Copy link
Collaborator

Thanks, @Phasip , for your response, I'm investigating.

@zinduolis
Copy link
Collaborator

Hi @Phasip , I've just tried on my Linux Ubuntu machine the following:

  1. git checkout master (going to my local master branch)
  2. git fetch upstream (get latest updates)
  3. git merge upstream/master (update my local master with master of the beef repo)
  4. git push (update my forked repository)
  5. git checkout -b testing_current_state (create branch for testing)
  6. sudo ./install (refresh the installation from the latest code / dependencies)
  7. bundle exec rake (run tests locally) - tests were successful
  8. ./beef (start beef) - beef started successfully without any errors
  9. successfully logged into beef and hooked the Firefox browser
  10. successfully executed Debug>Return Image command.

Your experience in comparison to mine tells me that most probably the rebase hasn't been successful. For the context, I have recently made some changes that fixed Browserstack tests.

Could you please try refreshing your local master with the latest changes (steps 1 to 4), test to make sure everything is working (steps 5 to 10) and then rebase your change branch with up to date master?

Thanks

@stephenakq stephenakq self-requested a review November 12, 2024 18:24
@Phasip
Copy link
Author

Phasip commented Nov 15, 2024

The rake issue was resolved by a missing dependency not being installed by ./install or the Dockerfile:
apt install ruby-espeak

(Note wget is also a dependency for running rake, but it is not installed in the Dockerfile nor ./install)

I don't think I can solve the BrowserStack issue

Copy link
Contributor

Stale pull request message

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