Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
"Self" message are not supported, but this was not checked. This
triggered the error "Something is wrong", and then caused a buffer
overflow.

Bug:#65
  • Loading branch information
ArthurSonzogni committed May 8, 2023
1 parent a97c425 commit 01ac869
Show file tree
Hide file tree
Showing 9 changed files with 72 additions and 11 deletions.
6 changes: 5 additions & 1 deletion CHANGELOG
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
# 1.0.148
# Current

## Security:
- See CVE-2023-27390. Check there are no "self" message.

## Build
- Set MSVC_RUNTIME_LIBRARY to /MT for static builds
5 changes: 5 additions & 0 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,11 @@ project(Diagon

option(DIAGON_BUILD_TESTS "Set to ON to build tests" OFF)
option(DIAGON_BUILD_TESTS_FUZZER "Set to ON to enable fuzzing" OFF)
option(DIAGON_ASAN "Set to ON to enable address sanitizer" OFF)
option(DIAGON_LSAN "Set to ON to enable leak sanitizer" OFF)
option(DIAGON_MSAN "Set to ON to enable memory sanitizer" OFF)
option(DIAGON_TSAN "Set to ON to enable thread sanitizer" OFF)
option(DIAGON_UBSAN "Set to ON to enable undefined behavior sanitizer" OFF)

include(FetchContent)
set(FETCHCONTENT_QUIET FALSE)
Expand Down
4 changes: 2 additions & 2 deletions cmake/diagon_fuzzer.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ set(CMAKE_C_COMPILER clang)
set(CMAKE_CXX_COMPILER clang++)

add_executable(fuzzer src/fuzzer.cpp)
target_compile_options(fuzzer PRIVATE -fsanitize=fuzzer,address)
target_link_libraries(fuzzer PRIVATE -fsanitize=fuzzer,address)
target_compile_options(fuzzer PRIVATE -fsanitize=fuzzer)
target_link_libraries(fuzzer PRIVATE -fsanitize=fuzzer)
target_link_libraries(fuzzer PRIVATE diagon_lib)
target_set_common(fuzzer)
39 changes: 39 additions & 0 deletions cmake/diagon_test.cmake
Original file line number Diff line number Diff line change
@@ -1,3 +1,42 @@
add_executable(input_output_test src/input_output_test.cpp)
target_link_libraries(input_output_test diagon_lib)
target_set_common(input_output_test)

option(DIAGON_ASAN "Set to ON to enable address sanitizer" OFF)
option(DIAGON_LSAN "Set to ON to enable leak sanitizer" OFF)
option(DIAGON_MSAN "Set to ON to enable memory sanitizer" OFF)
option(DIAGON_TSAN "Set to ON to enable thread sanitizer" OFF)
option(DIAGON_UBSAN "Set to ON to enable undefined behavior sanitizer" OFF)
if (NOT CMAKE_CXX_COMPILER_ID MATCHES "Clang")
return()
endif()

# Add ASAN
if (DIAGON_ASAN)
add_compile_options(-fsanitize=address)
add_link_options(-fsanitize=address)
endif()

# Add LSAN
if (DIAGON_LSAN)
add_compile_options(-fsanitize=leak)
add_link_options(-fsanitize=leak)
endif()

# Add MSAN
if (DIAGON_MSAN)
add_compile_options(-fsanitize=memory)
add_link_options(-fsanitize=memory)
endif()

# Add TSAN
if (DIAGON_TSAN)
add_compile_options(-fsanitize=thread)
add_link_options(-fsanitize=thread)
endif()

# Add UBSAN
if (DIAGON_UBSAN)
add_compile_options(-fsanitize=undefined)
add_link_options(-fsanitize=undefined)
endif()
3 changes: 2 additions & 1 deletion src/input_output_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -58,13 +58,14 @@ int main(int, const char**) {
continue;
}

// std::cout << " [RUN ] " << test.path() << std::endl;
std::string input = ReadFile(test.path() / "input");
std::string output = ReadFile(test.path() / "output");

std::string output_computed = translator->Translate(input, options);

if (output_computed == output) {
//std::cout << " [PASS] " << test.path() << std::endl;
std::cout << " [PASS] " << test.path() << std::endl;
} else {
std::cout << " [FAIL] " << test.path() << std::endl;
std::cout << "---[Output]------------------" << std::endl;
Expand Down
21 changes: 15 additions & 6 deletions src/translator/sequence/Sequence.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -288,6 +288,7 @@ std::string Sequence::Translate(const std::string& input,
UniformizeInternalRepresentation();
if (actors.size() == 0)
return "";

SplitByBackslashN();
Layout();
return Draw();
Expand Down Expand Up @@ -361,7 +362,7 @@ void Sequence::UniformizeMessageID() {
for (auto& message : messages) {
if (message.id != -1) {
if (used.count(message.id)) {
std::cout << "Found two messages with the same id: " << message.id
std::cerr << "Found two messages with the same id: " << message.id
<< std::endl;
message.id = -1;
} else {
Expand All @@ -385,21 +386,21 @@ void Sequence::UniformizeMessageID() {
for (int id : {dependency.from, dependency.to}) {
auto it = message_index.find(id);
if (it == message_index.end()) {
std::wcout << "* Ignored dependency: \"" << actor.name << ": "
std::wcerr << "* Ignored dependency: \"" << actor.name << ": "
<< dependency.from << " < " << dependency.to << "\"."
<< std::endl;
std::wcout << " It cannot be used because the message ID \"" << id
std::wcerr << " It cannot be used because the message ID \"" << id
<< "\" doesn't exist" << std::endl;
return true;
}

const Message& message = messages[it->second];
if (actor.name != message.from && //
actor.name != message.to) {
std::wcout << "* Ignored dependency: \"" << actor.name << ": "
std::wcerr << "* Ignored dependency: \"" << actor.name << ": "
<< dependency.from << " < " << dependency.to << "\"."
<< std::endl;
std::wcout << " It cannot be used because the message \""
std::wcerr << " It cannot be used because the message \""
<< message.from << " -> " << message.to << ": "
<< message.messages[0]
<< "\" has nothing to do with actor " << actor.name
Expand Down Expand Up @@ -464,6 +465,14 @@ void Sequence::AddMessageCommand(
message.from = GetText(message_command->text(0));
message.to = GetText(message_command->text(1));

if (message.from == message.to) {
std::cerr << "Self messages are not supported yet. It has been ignored."
<< std::endl;
std::cerr << "See https://github.com/ArthurSonzogni/Diagon/issues/63"
<< std::endl;
return;
}

if (message_command->arrow()->ARROW_LEFT()) {
std::swap(message.from, message.to);
}
Expand Down Expand Up @@ -553,7 +562,7 @@ void Sequence::LayoutComputeActorsPositions() {
}
}
if (i++ > 500) {
std::cout << "Something went wrong!" << std::endl;
std::cerr << "Something went wrong!" << std::endl;
break;
}
}
Expand Down
4 changes: 3 additions & 1 deletion src/translator/sequence/Sequence.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -95,11 +95,13 @@ class Sequence : public Translator {
int GetNumber(SequenceParser::NumberContext* number);
std::wstring GetText(SequenceParser::TextContext* text);

// 1.1) Check input validity.
bool ContainsSelfMessage();

// 2) Clean the representation.
void UniformizeInternalRepresentation();
void UniformizeActors();
void UniformizeMessageID();

void SplitByBackslashN();

// 3) Layout.
Expand Down
1 change: 1 addition & 0 deletions test/Sequence/cve-2023-27390/input
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
A->A:POC_BOF
Empty file.

0 comments on commit 01ac869

Please sign in to comment.