Skip to content

Commit

Permalink
[shell] Fix buffer overrun issue NCC-E003350-ABV. (#19533)
Browse files Browse the repository at this point in the history
* [shell] Fix buffer overrun issue NCC-E003350-ABV.

* [shell] Restore unit tests for TokenizeLine, and add test for max tokens.

* [shell] NULL to nullptr.

* formatting
  • Loading branch information
turon authored and pull[bot] committed Jun 25, 2022
1 parent 86bb799 commit 1736121
Show file tree
Hide file tree
Showing 8 changed files with 169 additions and 79 deletions.
5 changes: 5 additions & 0 deletions src/lib/shell/MainLoopCYW30739.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,11 @@ int TokenizeLine(char * buffer, char ** tokens, int max_tokens)
}
}
}
// If for too many arguments, overwrite last entry with guard.
if (cursor >= max_tokens)
{
cursor = max_tokens - 1;
}

tokens[cursor] = nullptr;

Expand Down
5 changes: 5 additions & 0 deletions src/lib/shell/MainLoopDefault.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,11 @@ int TokenizeLine(char * buffer, char ** tokens, int max_tokens)
}
}
}
// If for too many arguments, overwrite last entry with guard.
if (cursor >= max_tokens)
{
cursor = max_tokens - 1;
}

tokens[cursor] = nullptr;

Expand Down
5 changes: 5 additions & 0 deletions src/lib/shell/MainLoopEFR32.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,11 @@ int TokenizeLine(char * buffer, char ** tokens, int max_tokens)
}
}
}
// If for too many arguments, overwrite last entry with guard.
if (cursor >= max_tokens)
{
cursor = max_tokens - 1;
}

tokens[cursor] = nullptr;

Expand Down
8 changes: 3 additions & 5 deletions src/lib/shell/tests/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,9 @@ import("${chip_root}/build/chip/chip_test_suite.gni")
chip_test_suite("tests") {
output_name = "libTestShell"

sources = [
"TestStreamerStdio.cpp",
"TestStreamerStdio.h",
test_sources = [
"TestShellStreamerStdio.cpp",
"TestShellTokenizeLine.cpp",
]

cflags = [ "-Wconversion" ]
Expand All @@ -33,6 +33,4 @@ chip_test_suite("tests") {
"${chip_root}/src/lib/shell",
"${nlunit_test_root}:nlunit-test",
]

tests = [ "TestStreamerStdio" ]
}
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
* limitations under the License.
*/

#include "TestStreamerStdio.h"
#include <nlunit-test.h>

#include <lib/shell/Engine.h>
#include <lib/support/CodeUtils.h>
Expand Down Expand Up @@ -75,14 +75,14 @@ static void TestStreamer_Output(nlTestSuite * inSuite, void * inContext)
*/
static const nlTest sTests[] = {

NL_TEST_DEF("Test Streamer: TestStreamer_Output", TestStreamer_Output),
NL_TEST_DEF("Test Shell: TestStreamer_Output", TestStreamer_Output),

NL_TEST_SENTINEL()
};

int TestStreamerStdio(void)
{
nlTestSuite theSuite = { "CHIP Streamer tests", &sTests[0], nullptr, nullptr };
nlTestSuite theSuite = { "Test Shell: Streamer", &sTests[0], nullptr, nullptr };

// Run test suit againt one context.
nlTestRunner(&theSuite, nullptr);
Expand Down
148 changes: 148 additions & 0 deletions src/lib/shell/tests/TestShellTokenizeLine.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,148 @@
/*
*
* Copyright (c) 2020 Project CHIP Authors
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

#include <nlunit-test.h>

#include <lib/support/CodeUtils.h>
#include <lib/support/UnitTestRegistration.h>

// Include entire C++ file to have access to functions-under-test
// such as TokenizeLine despite them being declared within an anonymous namespace.
#include "../MainLoopDefault.cpp"

using namespace chip;
using namespace chip::Shell;
using namespace chip::Logging;

constexpr int TEST_SHELL_MAX_TOKENS = 5;

// =================================
// Test Vectors
// =================================

struct test_shell_vector
{
const char * line;
const char ** argv;
int argc;
};

static const struct test_shell_vector test_vector_shell_tokenizer[] = {
{
.line = "hello how are you?",
.argv = (const char *[]){ "hello", "how", "are", "you?" },
.argc = 4,
},
{
.line = "hey yo yo",
.argv = (const char *[]){ "hey", "yo", "yo" },
.argc = 3,
},
{
.line = "This has double spaces",
.argv = (const char *[]){ "This", "has", "double", "spaces" },
.argc = 4,
},
{
.line = " leading space",
.argv = (const char *[]){ "leading", "space" },
.argc = 2,
},
{
.line = "trailing space ",
.argv = (const char *[]){ "trailing", "space", "" },
.argc = 3,
},
{
.line = "no_space",
.argv = (const char *[]){ "no_space" },
.argc = 1,
},
{
.line = " ",
.argv = (const char *[]){},
.argc = 0,
},
{
.line = "",
.argv = (const char *[]){},
.argc = 0,
},
{
.line = "hey 1 2 3 4 5 6 7 max out",
.argv =
(const char *[]){
"hey",
"1",
"2",
"3",
},
.argc = TEST_SHELL_MAX_TOKENS - 1,
},
};

// =================================
// Unit tests
// =================================

static void TestShell_Tokenizer(nlTestSuite * inSuite, void * inContext)
{
int numOfTestVectors = ArraySize(test_vector_shell_tokenizer);
int numOfTestsRan = 0;
const struct test_shell_vector * test_params;

char line[128];

for (int vectorIndex = 0; vectorIndex < numOfTestVectors; vectorIndex++)
{
test_params = &test_vector_shell_tokenizer[vectorIndex];
strcpy(line, test_params->line);

char * argv[TEST_SHELL_MAX_TOKENS];
int argc = TokenizeLine(line, argv, TEST_SHELL_MAX_TOKENS);

NL_TEST_ASSERT(inSuite, argc == test_params->argc);

for (int i = 0; i < argc; i++)
{
NL_TEST_ASSERT(inSuite, strcmp(argv[i], test_params->argv[i]) == 0);
}
numOfTestsRan++;
}
NL_TEST_ASSERT(inSuite, numOfTestsRan > 0);
}

/**
* Test Suite. It lists all the test functions.
*/
static const nlTest sTests[] = {

NL_TEST_DEF("Test Shell: TestShell_Tokenizer", TestShell_Tokenizer),

NL_TEST_SENTINEL()
};

int TestShellTokenizeLine(void)
{
nlTestSuite theSuite = { "Test Shell: MainLoop", &sTests[0], nullptr, nullptr };

// Run test suit againt one context.
nlTestRunner(&theSuite, nullptr);
return nlTestRunnerStats(&theSuite);
}

CHIP_REGISTER_TEST_SUITE(TestShellTokenizeLine)
37 changes: 0 additions & 37 deletions src/lib/shell/tests/TestStreamerStdio.h

This file was deleted.

34 changes: 0 additions & 34 deletions src/lib/shell/tests/TestStreamerStdioDriver.cpp

This file was deleted.

0 comments on commit 1736121

Please sign in to comment.