Fix #1851 - Command/request(s) sent to zombie 'undefined' bug

[jackdwalker]

Pull Request

Thanks for submitting a PR! Please fill in this template where appropriate:

Category

e.g. Bug, Module, Extension, Core Functionality, Documentation, Tests
Bug

Feature/Issue Description

Q: Please give a brief summary of your feature/fix
A: The current release of BeEF master contains a bug in the 'Current Browser' zombie commands status bar. Any request made or command module launched resulted in an error similar to that highlighted in the picture below:

This patch resolves this issue.

Q: Give a technical rundown of what you have changed (if applicable)
A: The data previously passed into the constructor which generates these messages was only being given an object zombie which contained the one key session whose value was the Session ID for that hooked browser. The status bar was trying to call on that object to give a value for the key ip. I've edited the zombie object to contain the full context of the hooked browser which is open in the ZombieMgr panel. To do this I called on the beefwui (Beef Web UI?) API, and filtered for the hooked browser whose Session ID was in the URI fragment (location_hash('id')):

var zombie = Object.values(beefwui.hooked_browsers).find(hb => hb.session === id);

The location_hash('id') code is not mine, and was the previous way of identifying the Session ID (see the commit diff) of the hooked browser being examined.

Test Cases

Q: Describe your test cases, what you have covered and if there are any use cases that still need addressing.
A: None written - Admin UI extension test (and all other tests) still pass, and my manual testing of the UI worked :)

Wiki Page

If you are adding a new feature that is not easily understood without context, please draft a section to be added to the Wiki below.

N/A

[bcoles]

Using location_hash('id') is probably not the correct way to do this. Maybe. If I remember correctly, the current zombie tab is meant to be initialized with the zombie object somewhere. This object should contain the ID.

[jackdwalker]

No problems. I'll have a dig around for that object when I next get a chance and try to utilise that.

[wheatley]

@bcoles / @jackdwalker / @jcrew99 - was this completed?
I can see in the compare the zombie object is being used when initialising the ZombieTab object as requested by @bcoles

If this was the intended update, happy to merge this and add to Friday release

[xel-dev]

I don't believe any changes were made following my original PR @wheatley.

Re-reading with effectively zero context (given how long it has been lol), I believe @bcoles was implying that the zombie object should have already been initialized and be accessible with an ID object rather which would avoid the need to re-instantiate it as I did in my code.

That being said I don't recall the a higher order zombie object being available here although it's been forever so I'd need to take a look again.

I do remember this fixing the solution pretty nicely but I didn't get a chance to investigate @bcoles comment further at the time. So there's a chance this could introduce other issues he is aware of that I was not when developing this patch.

[bcoles]

Re-reading with effectively zero context (given how long it has been lol), I believe @bcoles was implying that the zombie object should have already been initialized and be accessible with an ID object rather which would avoid the need to re-instantiate it as I did in my code.

The original PR used this code:

  var zombie = Object.values(beefwui.hooked_browsers).find(hb => hb.session === location_hash('id'));

That relies on location_hash which I'm going to presume was the location.hash, ie the URL anchor after #.

This was changed in the Tiny refactor commit in this PR to:

  var zombie = Object.values(beefwui.hooked_browsers).find(hb => hb.session === id);

Which no longer makes use of the location hash.

I do remember this fixing the solution pretty nicely but I didn't get a chance to investigate @bcoles comment further at the time. So there's a chance this could introduce other issues he is aware of that I was not when developing this patch.

I haven't tested this PR but my comment was addressed.

[xel-dev]

The original PR used this code:

  var zombie = Object.values(beefwui.hooked_browsers).find(hb => hb.session === location_hash('id'));

That relies on location_hash which I'm going to presume was the location.hash, ie the URL anchor after #.

This was changed in the Tiny refactor commit in this PR to:

  var zombie = Object.values(beefwui.hooked_browsers).find(hb => hb.session === id);

Which no longer makes use of the location hash.

I haven't tested this PR but my comment was addressed.

Gotcha - makes sense. Looking back at the individual commit/comment timestamps it seems like I did end up updating the PR with that code the day you left that comment (as you've mentioned).

@wheatley - feel free to merge if it is passing tests and still works functionally.

[wheatley]

Had a quick look, looks good.

[wheatley]

Is this where the change for for the zombie object referred to by @bcoles?