rapid7 / metasploit-framework

Metasploit Framework
https://www.metasploit.com/
Other
33.4k stars 13.82k forks source link

A few nil bugs in Hardware Bridge #8022

Closed wvu closed 7 years ago

wvu commented 7 years ago

Ran into these while my vehicle was off. No problem while on.

I am using live Kali at the moment, so please forgive me if these have already been fixed since the last update. I will retest with my dev env once set up.

msf post(getvinfo) > run

[-] Post failed: NoMethodError undefined method `size' for nil:NilClass
[-] Call stack:
[-]   /usr/share/metasploit-framework/lib/rex/post/hwbridge/extensions/automotive/automotive.rb:60:in `check_for_errors'
[-]   /usr/share/metasploit-framework/lib/rex/post/hwbridge/extensions/automotive/automotive.rb:114:in `send_isotp_and_wait_for_response'
[-]   /usr/share/metasploit-framework/lib/msf/core/post/hardware/automotive/uds.rb:93:in `get_current_data'
[-]   /usr/share/metasploit-framework/lib/msf/core/post/hardware/automotive/uds.rb:106:in `get_current_data_pids'
[-]   /usr/share/metasploit-framework/modules/post/hardware/automotive/getvinfo.rb:35:in `run'
[*] Post module execution completed
msf post(getvinfo) > 
msf post(getvinfo) > run

[-] Post failed: NoMethodError undefined method `join' for nil:NilClass
[-] Call stack:
[-]   /usr/share/metasploit-framework/lib/msf/core/post/hardware/automotive/uds.rb:110:in `get_current_data_pids'
[-]   /usr/share/metasploit-framework/modules/post/hardware/automotive/getvinfo.rb:35:in `run'
[*] Post module execution completed
msf post(getvinfo) > 
msf post(getvinfo) > run

[-] Post failed: NoMethodError undefined method `[]' for nil:NilClass
[-] Call stack:
[-]   /usr/share/metasploit-framework/lib/msf/core/post/hardware/automotive/uds.rb:109:in `get_current_data_pids'
[-]   /usr/share/metasploit-framework/modules/post/hardware/automotive/getvinfo.rb:35:in `run'
[*] Post module execution completed
msf post(getvinfo) > 
hwbridge > cansend 
[-] Error running command cansend: NoMethodError private method `active_bus' called for #<Rex::Post::HWBridge::Ui::Console::CommandDispatcher::Automotive:0x00000013d98970>
Did you mean?  active_db?
hwbridge > 
hwbridge > supported_buses 
[-] Error running command supported_buses: NoMethodError undefined method `size' for nil:NilClass
hwbridge > 

Here's a good run (vehicle on):

msf post(getvinfo) > run

[*] Available PIDS for pulling realtime data: 40 pids
[*]   [1, 3, 4, 5, 6, 7, 11, 12, 13, 14, 15, 17, 19, 21, 28, 31, 32, 32, 33, 35, 46, 47, 48, 49, 50, 51, 52, 60, 64, 65, 66, 67, 68, 69, 70, 71, 73, 74, 76, 81]
[*]   MIL (Engine Light) : OFF
[*]   Number of DTCs: 0
[*]   Engine Temp: 88 °C / 190 °F
[*]   RPMS: 747
[*]   Speed: 0 km/h  /  0.0 mph
[*] Supported OBD Standards: OBD and OBD-II
[*] Mode $09 Vehicle Info Supported PIDS: [2, 4, 6, 8, 10]
[*] VIN: [redacted]
[*] Calibration ID: [redacted]
[*] PID 6 Response: ["01", "E6", "67", "9B", "45"]
[*] PID 8 Response: ["04", "2F", "09", "38", "03", "92", "04", "2F", "00", "00", "00", "00", "05", "26", "04", "2F", "00", "00", "00", "00", "07", "22", "04", "2F", "00", "00", "00", "00", "00", "CB", "01", "87", "04", "5A", "04", "2F", "00", "00", "00", "00", "00", "00", "00", "00", "00"]
[*] ECU Name: ECM-EngineControl
[*] Post module execution completed
msf post(getvinfo) > 

Fun stuff! Thanks for the great work, @zombieCraig. This sure opens up a lot of doors for us.

7795

rakesh-verma-16 commented 7 years ago

I'm a new contributor to the organisation and I'd like to be assigned to me. Any help on solving my first issue would also be very appreciated.

zombieCraig commented 7 years ago

@wvu-r7 when the car was off, was the bridge still running or was that closed as well? Currently there isn't a method to determine if the state between the relay and the hardware client is still active. I can of course clean up some of these null errors but I'm wondering if there is a bigger problem that needs to be addressed and some more information on the condition at the time of failure would help.

wvu commented 7 years ago

@zombieCraig: The bridge was still running, yeah. It's just empty packets being dereferenced. If there's a better way to check for that instead of on a case-by-case basis, that'd resolve the crux of this issue. I can gather more info later today. Thanks for the swift response!

zombieCraig commented 7 years ago

@wvu-r7 I found the core issue. The ELM327 reports CAN ERROR when sending packets and the relay was treating that as a valid CAN packet. Once I added a proper check for ELM327 so that it did not consider that valid CAN traffic (duh) then that fixed the null errors. I'll upload the fix once I clean up a few other things

wvu commented 7 years ago

Ahh, that makes sense. I haven't been deep in the code yet, so I can't comment on how anything is done. I'll be sure to test your fix when it's ready. Thank you!

I just realized how bad my bug report is, too. Will do better next time. Sorry about that.

h00die commented 7 years ago

this was fixed right, can we close it?

wvu commented 7 years ago

Not until #8031 lands. I'm still tracking down some remaining nil bugs. I'm seeing sporadic get_current_data_pids failures still.