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

Remove vusb 'reset to bootloader' hid message due to security implications #7456

Merged
merged 1 commit into from
Nov 25, 2019

Conversation

zvecr
Copy link
Member

@zvecr zvecr commented Nov 23, 2019

Description

A malicious actor could send a well crafted hid message to a vusb board, which resets to bootloader. They would then be able to automatically flash firmware, without user intervention, firmware which is potentially compromised.

Types of Changes

  • Core
  • Bugfix
  • New feature
  • Enhancement/optimization
  • Keyboard (addition or update)
  • Keymap/layout/userspace (addition or update)
  • Documentation

Checklist

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • I have tested the changes and verified that they work and don't break anything (as well as I can manage).

@zvecr zvecr added the core label Nov 23, 2019
@zvecr zvecr requested a review from a team November 23, 2019 18:13
@skullydazed
Copy link
Member

Some context- Since QMK is probably the most popular keyboard firmware we are the most likely platform to be targeted for attacks. With this code in place it's possible for a malicious actor to load compromised firmware into a user's keyboard without the user's knowledge.

@zvecr zvecr merged commit f0f161e into qmk:master Nov 25, 2019
ripxorip pushed a commit to ripxorip/qmk_firmware that referenced this pull request Dec 3, 2019
@fauxpark fauxpark mentioned this pull request Dec 11, 2019
13 tasks
patrl pushed a commit to patrl/qmk_firmware that referenced this pull request Dec 29, 2019
@zvecr zvecr mentioned this pull request Jan 3, 2020
13 tasks
HokieGeek pushed a commit to HokieGeek/qmk_firmware that referenced this pull request Feb 21, 2020
@yiancar
Copy link
Contributor

yiancar commented Mar 17, 2020

This is to go to bootloader from host yes?

@zvecr zvecr deleted the feature/vusb_remove_reboot branch April 28, 2020 01:03
@xyzz
Copy link
Contributor

xyzz commented Jun 28, 2020

Is there the same problem in VIA? Here:

qmk_firmware/quantum/via.c

Lines 377 to 385 in 0928496

case id_bootloader_jump: {
// Need to send data back before the jump
// Informs host that the command is handled
raw_hid_send(data, length);
// Give host time to read it
wait_ms(100);
bootloader_jump();
break;
}

BorisTestov pushed a commit to BorisTestov/qmk_firmware that referenced this pull request May 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants