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

Fixed bounds check of connectors when validating charging profile #925

Merged
merged 2 commits into from
Jan 4, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion lib/ocpp/v16/smart_charging.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,7 @@ bool SmartChargingHandler::validate_profile(
ChargingProfile& profile, const int connector_id, bool ignore_no_transaction, const int profile_max_stack_level,
const int max_charging_profiles_installed, const int charging_schedule_max_periods,
const std::vector<ChargingRateUnit>& charging_schedule_allowed_charging_rate_units) {
if ((size_t)connector_id > this->connectors.size() or connector_id < 0 or profile.stackLevel < 0 or
if ((size_t)connector_id >= this->connectors.size() or connector_id < 0 or profile.stackLevel < 0 or
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In general we should probably use static_cast here instead of using a c-style cast

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This has been addressed with #929

profile.stackLevel > profile_max_stack_level) {
EVLOG_warning << "INVALID PROFILE - connector_id invalid or invalid stack level";
return false;
Expand Down
10 changes: 9 additions & 1 deletion tests/lib/ocpp/v16/test_smart_charging_handler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -209,6 +209,7 @@ class ChargepointTestFixture : public testing::Test {
auto database = std::make_unique<common::DatabaseConnection>(database_path / (chargepoint_id + ".db"));
std::shared_ptr<DatabaseHandlerMock> database_handler =
std::make_shared<DatabaseHandlerMock>(std::move(database), init_script_path);
addConnector(0);
auto handler = new SmartChargingHandler(connectors, database_handler, *configuration);
return handler;
}
Expand Down Expand Up @@ -302,12 +303,19 @@ TEST_F(ChargepointTestFixture, ValidateProfile__ConnectorIdGreaterThanConnectors
const std::vector<ChargingRateUnit>& charging_schedule_allowed_charging_rate_units{ChargingRateUnit::A};
auto handler = createSmartChargingHandler();

const int connector_id = INT_MAX;
int connector_id = INT_MAX;
bool sut = handler->validate_profile(profile, connector_id, ignore_no_transaction, profile_max_stack_level,
max_charging_profiles_installed, charging_schedule_max_periods,
charging_schedule_allowed_charging_rate_units);

ASSERT_FALSE(sut);

// if we have connectors 0,1,2 this->connectors.size() == 3 and a connector_id of 3 is invalid
connector_id = this->connectors.size();
sut = handler->validate_profile(profile, connector_id, ignore_no_transaction, profile_max_stack_level,
max_charging_profiles_installed, charging_schedule_max_periods,
charging_schedule_allowed_charging_rate_units);
ASSERT_FALSE(sut);
}

/**
Expand Down