8000 COMM_WRITE_UNLOCK_CMD and vulnerbility fixes by Relys · Pull Request #4 · surfdado/bldc · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

COMM_WRITE_UNLOCK_CMD and vulnerbility fixes #4

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.

Sign up for GitHub

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

Open
wants to merge 2 commits into
base: pin_lock
Choose a base branch
from
Open

COMM_WRITE_UNLOCK_CMD and vulnerbility fixes #4

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
fa106a6
COMM_WRITE_UNLOCK_CMD and vulnerbility fixes
Relys Nov 4, 2023
740750e
Don't increase cooldown for the same pin.
Relys Nov 15, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
128 changes: 108 additions & 20 deletions comm/commands.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,14 @@ static thread_t *blocking_tp;
static bool is_lock_initialized = false;
static bool writelock = false;//true;
static unsigned int writelock_pin = 0;
static unsigned int writelock_pin_prev = 0;
// Track last time there was a incorrect pin submitted to protect against bruteforce attemps.
static systime_t writelock_last_failed_pin_attempt = 0;
static unsigned int writelock_pin_attempt_cooldown=1000;
static bool writelock_disabled_last_cmd = false;
// Protect against stack based overflow in COMM_FORWARD_CAN and COMM_WRITE_UNLOCK_CMD
static int recursion_depth = 0;
static const int MAX_RECURSION_DEPTH = 5; // Set a limit for recursion depth

// Private variables
static char print_buffer[PRINT_BUFFER_SIZE];
Expand Down Expand Up @@ -244,7 +252,8 @@ void commands_process_packet(unsigned char *data, unsigned int len,
(packet_id != COMM_GET_CUSTOM_CONFIG) &&
(packet_id != COMM_CUSTOM_APP_DATA) &&
(packet_id != COMM_LOCK_STATUS) &&
(packet_id != COMM_WRITE_LOCK)) {
(packet_id != COMM_WRITE_LOCK) &&
(packet_id != COMM_WRITE_UNLOCK_CMD)) {
//commands_printf("Blocked command: ID %d\n", packet_id);
return;
}
Expand All @@ -255,7 +264,7 @@ void commands_process_packet(unsigned char *data, unsigned int len,
uint32_t odo_now = mc_interface_get_odometer();
// Writing the current odometer value removes the writelock
if (abs(odo_now-odo_new) < 500) {
// the VESC App only allows setting of odometer in km/mi not in meters
// the VESC App only allows setting of odometer in km/mi not in meters
writelock = false;
}
return;
Expand All @@ -270,8 +279,8 @@ void commands_process_packet(unsigned char *data, unsigned int len,
}

}

switch (packet_id) {
switch (packet_id) {
case COMM_FW_VERSION: {
int32_t ind = 0;
uint8_t send_buffer[65];
Expand Down Expand Up @@ -777,12 +786,19 @@ void commands_process_packet(unsigned char *data, unsigned int len,
} break;

case COMM_FORWARD_CAN: {
if (len <= 1 || recursion_depth >= MAX_RECURSION_DEPTH) {
// Base case: Not enough data to process or max recursion depth reached
recursion_depth = 0; // Reset recursion depth
return;
}
send_func_can_fwd = reply_func;

#ifdef HW_HAS_DUAL_MOTORS
if (data[0] == utils_second_motor_id()) {
mc_interface_select_motor_thread(2);
recursion_depth++;
commands_process_packet(data + 1, len - 1, reply_func);
recursion_depth--;
mc_interface_select_motor_thread(1);
} else {
comm_can_send_buffer(data[0], data + 1, len - 1, 0);
Expand Down Expand Up @@ -1698,12 +1714,21 @@ void commands_process_packet(unsigned char *data, unsigned int len,
uint8_t magic_number = data[ind++];
uint32_t pin = buffer_get_uint16(data, &ind);
uint8_t lock_enable = data[ind++];

if ((magic_number == 169) && (pin == writelock_pin)) {
systime_t current_time = chVTGetSystemTimeX();
if ((magic_number == 169) && (pin == writelock_pin) && (writelock_last_failed_pin_attempt == 0 || ((current_time - writelock_last_failed_pin_attempt) < writelock_pin_attempt_cooldown))) {
writelock = (lock_enable && pin>0) ? true : false;
writelock_last_failed_pin_attempt = 0;
writelock_disabled_last_cmd=!writelock;
break;
}
if(pin != writelock_pin_prev)
{
writelock_pin_attempt_cooldown*=2;
writelock_last_failed_pin_attempt = chVTGetSystemTimeX();
}
// Sends no response - call COMM_LOCK_STATUS to check success/fail
writelock_pin_prev = pin;
}
// Sends no response - call COMM_LOCK_STATUS to check success/fail
} break;

case COMM_LOCK_SETPIN: {
Expand All @@ -1715,18 +1740,25 @@ void commands_process_packet(unsigned char *data, unsigned int len,
bool lock_on_boot = (data[ind++] == 1);

if (magic_number == 169) {
int didset = 0;
if (old_pin == writelock_pin) {
// write new pin to eeprom
conf_general_set_writelock_pin(new_pin, lock_on_boot);
writelock_pin = conf_general_get_writelock_pin();

// when lock_on_boot is set, we immediately enable writelock
writelock = (writelock_pin > 0) && lock_on_boot;
didset = 1;
}
int didset = 0;
systime_t current_time = chVTGetSystemTimeX();
if (old_pin == writelock_pin && (writelock_last_failed_pin_attempt == 0 || ((current_time - writelock_last_failed_pin_attempt) < writelock_pin_attempt_cooldown))) {
// write new pin to eeprom
conf_general_set_writelock_pin(new_pin, lock_on_boot);
writelock_pin = conf_general_get_writelock_pin();
// when lock_on_boot is set, we immediately enable writelock
writelock = (writelock_pin > 0) && lock_on_boot;
didset = 1;
writelock_last_failed_pin_attempt = 0;
}
else {
new_pin = 0;
if(old_pin != writelock_pin_prev)
{
writelock_pin_attempt_cooldown*=2;
writelock_last_failed_pin_attempt = chVTGetSystemTimeX();
}
writelock_pin_prev = old_pin;
}
ind = 0;

Expand Down Expand Up @@ -1754,7 +1786,21 @@ void commands_process_packet(unsigned char *data, unsigned int len,
send_buffer[ind++] = packet_id;
send_buffer[ind++] = 169; // magic number!
send_buffer[ind++] = writelock; // is writelock currently in place?
send_buffer[ind++] = (pin == writelock_pin); // does the passed pin match?
systime_t current_time = chVTGetSystemTimeX();
if(writelock_last_failed_pin_attempt == 0 || ((current_time - writelock_last_failed_pin_attempt) < writelock_pin_attempt_cooldown)){
send_buffer[ind++] = (pin == writelock_pin); // does the passed pin match?
writelock_last_failed_pin_attempt = 0;
}
else
{
send_buffer[ind++] = 0; // protection against bruteforce attacks
if(pin != writelock_pin_prev)
{
writelock_pin_attempt_cooldown*=2;
writelock_last_failed_pin_attempt = chVTGetSystemTimeX();
}
writelock_pin_prev = pin;
}
send_buffer[ind++] = (writelock_pin != 0); // is a pin set?
send_buffer[ind++] = conf_general_is_locked_on_boot();

Expand All @@ -1765,6 +1811,48 @@ void commands_process_packet(unsigned char *data, unsigned int len,
}
} break;

//Allows a command to be called while writelock is enabled by passing in the pin and command to be run.
case COMM_WRITE_UNLOCK_CMD:{
if (len <= 2 || recursion_depth >= MAX_RECURSION_DEPTH) {
// Base case: Not enough data to process or max recursion depth reached
recursion_depth = 0; // Reset recursion depth
return;
}
int32_t ind = 0;
uint32_t pin = buffer_get_uint16(data, &ind);
if(writelock){
systime_t current_time = chVTGetSystemTimeX();
if(pin == writelock_pin && (writelock_last_failed_pin_attempt == 0 || ((current_time - writelock_last_failed_pin_attempt) > writelock_pin_attempt_cooldown))){
writelock=false;
writelock_disabled_last_cmd=false;
recursion_depth++;
commands_process_packet(data + 2, len - 2, reply_func);
recursion_depth--;
if(!writelock_disabled_last_cmd) //special case if we call COMM_WRITE_LOCK and disable the lock
{
writelock=true;
}
writelock_disabled_last_cmd=false;
writelock_last_failed_pin_attempt = 0;
}
else
{
// Sends no response - call COMM_LOCK_STATUS to check success/fail
if(pin != writelock_pin_prev)
{
writelock_pin_attempt_cooldown*=2;
writelock_last_failed_pin_attempt = chVTGetSystemTimeX();
}
writelock_pin_prev = pin;
}
break;
}
//process the command as normal if writelock isn't enabled
recursion_depth++;
commands_process_packet(data + 2, len - 2, reply_func);
recursion_depth--;
} break;

// Blocking commands. Only one of them runs at any given time, in their
// own thread. If other blocking commands come before the previous one has
// finished, they are discarded.
Expand Down Expand Up @@ -1801,7 +1889,7 @@ void commands_process_packet(unsigned char *data, unsigned int len,
default:
break;
}
}
}

int commands_printf(const char* format, ...) {
chMtxLock(&print_mutex);
Expand Down Expand Up @@ -1997,7 +2085,7 @@ void commands_apply_mcconf_hw_limits(mc_configuration *mcconf) {
}

utils_truncate_number_abs(&mcconf->foc_sl_erpm_start, mcconf->foc_sl_erpm * 0.9);

#ifndef DISABLE_HW_LIMITS

// TODO: Maybe truncate values that get close to numerical instabilities when set
Expand Down
1 change: 1 addition & 0 deletions datatypes.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1157,6 +1157,7 @@ typedef enum {
COMM_LOCK_SETPIN,
COMM_WRITE_LOCK,
COMM_LOCK_STATUS,
COMM_WRITE_UNLOCK_CMD,
} COMM_PACKET_ID;

// CAN commands
Expand Down
0