Skip to content

Commit 6a684bf

Browse files
committed
fix(modem): Harden AT library against command injection
1 parent 70d42b7 commit 6a684bf

File tree

1 file changed

+26
-20
lines changed

1 file changed

+26
-20
lines changed

components/esp_modem/src/esp_modem_command_library.cpp

Lines changed: 26 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* SPDX-FileCopyrightText: 2021-2025 Espressif Systems (Shanghai) CO LTD
2+
* SPDX-FileCopyrightText: 2021-2026 Espressif Systems (Shanghai) CO LTD
33
*
44
* SPDX-License-Identifier: Apache-2.0
55
*/
@@ -89,10 +89,9 @@ template <typename T> command_result generic_get_string(CommandableIf *t, const
8989
std::string_view response((char *)data, len);
9090
while ((pos = response.find('\n')) != std::string::npos) {
9191
std::string_view token = response.substr(0, pos);
92-
for (auto it = token.end() - 1; it > token.begin(); it--) // strip trailing CR or LF
93-
if (*it == '\r' || *it == '\n') {
94-
token.remove_suffix(1);
95-
}
92+
while (!token.empty() && (token.back() == '\r' || token.back() == '\n')) {
93+
token.remove_suffix(1);
94+
}
9695
ESP_LOGV(TAG, "Token: {%.*s}\n", static_cast<int>(token.size()), token.data());
9796

9897
if (token.find("OK") != std::string::npos) {
@@ -187,7 +186,7 @@ command_result get_battery_status(CommandableIf *t, int &voltage, int &bcs, int
187186
out = out.substr(pattern.size());
188187
int pos, value, property = 0;
189188
while ((pos = out.find(',')) != std::string::npos) {
190-
if (std::from_chars(out.data(), out.data() + pos, value).ec == std::errc::invalid_argument) {
189+
if (std::from_chars(out.data(), out.data() + pos, value).ec != std::errc{}) {
191190
return command_result::FAIL;
192191
}
193192
switch (property++) {
@@ -200,7 +199,7 @@ command_result get_battery_status(CommandableIf *t, int &voltage, int &bcs, int
200199
}
201200
out = out.substr(pos + 1);
202201
}
203-
if (std::from_chars(out.data(), out.data() + out.size(), voltage).ec == std::errc::invalid_argument) {
202+
if (std::from_chars(out.data(), out.data() + out.size(), voltage).ec != std::errc{}) {
204203
return command_result::FAIL;
205204
}
206205
return command_result::OK;
@@ -224,10 +223,13 @@ command_result get_battery_status_sim7xxx(CommandableIf *t, int &voltage, int &b
224223
}
225224

226225
int volt, fraction;
227-
if (std::from_chars(out.data() + num_pos, out.data() + dot_pos, volt).ec == std::errc::invalid_argument) {
226+
if (std::from_chars(out.data() + num_pos, out.data() + dot_pos, volt).ec != std::errc{}) {
227+
return command_result::FAIL;
228+
}
229+
if (dot_pos + 2 > out.size()) {
228230
return command_result::FAIL;
229231
}
230-
if (std::from_chars(out.data() + dot_pos + 1, out.data() + out.size() - 1, fraction).ec == std::errc::invalid_argument) {
232+
if (std::from_chars(out.data() + dot_pos + 1, out.data() + out.size() - 1, fraction).ec != std::errc{}) {
231233
return command_result::FAIL;
232234
}
233235
bcl = bcs = -1; // not available for these models
@@ -256,14 +258,14 @@ command_result get_operator_name(CommandableIf *t, std::string &operator_name, i
256258
if (property++ == 2) { // operator name is after second comma (as a 3rd property of COPS string)
257259
operator_name = out.substr(++pos);
258260
auto additional_comma = operator_name.find(','); // check for the optional ACT
259-
if (additional_comma != std::string::npos && std::from_chars(operator_name.data() + additional_comma + 1, operator_name.data() + operator_name.length(), act).ec != std::errc::invalid_argument) {
261+
if (additional_comma != std::string::npos && std::from_chars(operator_name.data() + additional_comma + 1, operator_name.data() + operator_name.length(), act).ec == std::errc{}) {
260262
operator_name = operator_name.substr(0, additional_comma);
261263
}
262264
// and strip quotes if present
263265
auto quote1 = operator_name.find('"');
264266
auto quote2 = operator_name.rfind('"');
265-
if (quote1 != std::string::npos && quote2 != std::string::npos) {
266-
operator_name = operator_name.substr(quote1 + 1, quote2 - 1);
267+
if (quote1 != std::string::npos && quote2 != std::string::npos && quote2 > quote1) {
268+
operator_name = operator_name.substr(quote1 + 1, quote2 - quote1 - 1);
267269
}
268270
return command_result::OK;
269271
}
@@ -447,10 +449,10 @@ command_result get_signal_quality(CommandableIf *t, int &rssi, int &ber)
447449
return command_result::FAIL;
448450
}
449451

450-
if (std::from_chars(out.data() + rssi_pos, out.data() + ber_pos, rssi).ec == std::errc::invalid_argument) {
452+
if (std::from_chars(out.data() + rssi_pos, out.data() + ber_pos, rssi).ec != std::errc{}) {
451453
return command_result::FAIL;
452454
}
453-
if (std::from_chars(out.data() + ber_pos + 1, out.data() + out.size(), ber).ec == std::errc::invalid_argument) {
455+
if (std::from_chars(out.data() + ber_pos + 1, out.data() + out.size(), ber).ec != std::errc{}) {
454456
return command_result::FAIL;
455457
}
456458
return command_result::OK;
@@ -482,7 +484,7 @@ command_result get_network_attachment_state(CommandableIf *t, int &state)
482484
return command_result::FAIL;
483485
}
484486

485-
if (std::from_chars(out.data() + pos, out.data() + out.size(), state).ec == std::errc::invalid_argument) {
487+
if (std::from_chars(out.data() + pos, out.data() + out.size(), state).ec != std::errc{}) {
486488
return command_result::FAIL;
487489
}
488490

@@ -509,7 +511,7 @@ command_result get_radio_state(CommandableIf *t, int &state)
509511
return command_result::FAIL;
510512
}
511513

512-
if (std::from_chars(out.data() + pos, out.data() + out.size(), state).ec == std::errc::invalid_argument) {
514+
if (std::from_chars(out.data() + pos, out.data() + out.size(), state).ec != std::errc{}) {
513515
return command_result::FAIL;
514516
}
515517

@@ -570,12 +572,16 @@ command_result get_network_system_mode(CommandableIf *t, int &mode)
570572
}
571573

572574
constexpr std::string_view pattern = "+CNSMOD: ";
573-
int mode_pos = out.find(",") + 1; // Skip "<n>,"
574575
if (out.find(pattern) == std::string::npos) {
575576
return command_result::FAIL;
576577
}
578+
auto comma = out.find(',');
579+
if (comma == std::string::npos) {
580+
return command_result::FAIL;
581+
}
582+
size_t mode_pos = comma + 1;
577583

578-
if (std::from_chars(out.data() + mode_pos, out.data() + out.size(), mode).ec == std::errc::invalid_argument) {
584+
if (std::from_chars(out.data() + mode_pos, out.data() + out.size(), mode).ec != std::errc{}) {
579585
return command_result::FAIL;
580586
}
581587

@@ -602,7 +608,7 @@ command_result get_gnss_power_mode(CommandableIf *t, int &mode)
602608
return command_result::FAIL;
603609
}
604610

605-
if (std::from_chars(out.data() + pos, out.data() + out.size(), mode).ec == std::errc::invalid_argument) {
611+
if (std::from_chars(out.data() + pos, out.data() + out.size(), mode).ec != std::errc{}) {
606612
return command_result::FAIL;
607613
}
608614

@@ -668,7 +674,7 @@ command_result get_network_registration_state(CommandableIf *t, int &state)
668674
}
669675

670676
// Extract state value (skip the comma)
671-
if (std::from_chars(out.data() + state_pos_start + 1, out.data() + state_pos_end, state).ec == std::errc::invalid_argument) {
677+
if (std::from_chars(out.data() + state_pos_start + 1, out.data() + state_pos_end, state).ec != std::errc{}) {
672678
return command_result::FAIL;
673679
}
674680

0 commit comments

Comments
 (0)