ARMmbed / mbed-os

Arm Mbed OS is a platform operating system designed for the internet of things
https://mbed.com
Other
4.67k stars 2.98k forks source link

unsafe strcpy() used in MbedOS Client Cli library #11806

Closed TheSilentDawn closed 4 years ago

TheSilentDawn commented 4 years ago

Description of defect

Reference: https://github.com/ARMmbed/mbed-os/tree/master/features/frameworks/mbed-client-cli Function: cmd_run https://github.com/ARMmbed/mbed-os/blob/d91ed5fa42ea0f32e4422a3c562e7b045a17da40/features/frameworks/mbed-client-cli/source/ns_cmdline.c#L601 Type: Buffer overflow The function cmd_push() is used to split a list of commands donated as cmd_str into separate commands. The delimiter is a semicolon in this case. Each command is then copied into a string array cmd_ptr. However, if the command does not terminate with a null character, the strcpy will cause a buffer overflow as shown in line 4.

static void cmd_push(char *cmd_str, operator_t oper)
{
...
    strcpy(cmd_ptr->cmd_s, cmd_str);
...
}

Result: Memory corruption.

Target(s) affected by this defect ?

MbedOS Client Cli library latest

Toolchain(s) (name and version) displaying this defect ?

N/A

What version of Mbed-os are you using (tag or sha) ?

MbedOS 5.13.2

What version(s) of tools are you using. List all that apply (E.g. mbed-cli)

mbed-cli latest version

How is this defect reproduced ?

Use bug_cli_2 as input to demo codes below

// The code is based on the demo in the CoAP library source code of MbedOS
cmd_init( &myprint );              // initialize cmdline with print function
cmd_set_ready_cb( cmd_ready_cb );  // configure ready cb
cmd_add("dummy", cmd_dummy, 0, 0); // add one dummy command
cmd_add("long", cmd_long, 0, 0);   // add one dummy command
uint16_t cmd_len = 0;
read(0, &cmd_len, sizeof(cmd_len));
char* p = (char*)malloc(cmd_len);
read(0, p, cmd_len);
cmd_exe(p);

bug_cli_2.log

adbridge commented 4 years ago

No longer valid with the move to a new CI