Skip to content

Commit 0841be0

Browse files
authored
[posix] detect and fail on unused radio URL parameters (openthread#13087)
This commit enhances the radio URL parsing logic to detect and fail when unused parameters are provided in the URL. This prevents typos or unsupported parameters from being silently ignored. The following changes were made: - Updated ot::Url::Url to track parameter usage by appending a trailing '&' delimiter in Init() and replacing it with '\0' in GetValue() when a parameter is matched. This marks the parameter as used and removes any limit on the number of trackable parameters. - Added a Validate() method to ot::Url::Url to verify that all parameters in the query string were accessed. - Refactored ot::Posix::Radio to share a single RadioUrl instance with SpinelManager, ensuring all components track usage on the same URL object. - Integrated Validate() calls in otSysInit() and platformTrelInit() to perform validation after all platform components have been initialized. - Updated Radio::ProcessMaxPowerTable to use a local copy of the parameter string to avoid premature modification of the URL buffer. - Adjusted RadioUrl and unit tests to provide sufficient buffer space for the additional tracking delimiter. - Added new unit tests in tests/unit/test_url.cpp to verify the usage tracking and validation logic.
1 parent 545a83e commit 0841be0

10 files changed

Lines changed: 233 additions & 68 deletions

File tree

src/lib/url/url.cpp

Lines changed: 79 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,7 @@ otError Url::Init(char *aUrl)
4949
{
5050
otError error = OT_ERROR_NONE;
5151
char *url = aUrl;
52+
char *query;
5253

5354
mEnd = aUrl + strlen(aUrl);
5455
mProtocol = aUrl;
@@ -59,15 +60,31 @@ otError Url::Init(char *aUrl)
5960
url += sizeof("://") - 1;
6061
mPath = url;
6162

62-
url = strstr(url, "?");
63+
query = strchr(url, '?');
6364

64-
if (url != nullptr)
65+
if (query != nullptr)
6566
{
66-
mQuery = ++url;
67+
char *end;
6768

68-
for (char *cur = strtok(url, "&"); cur != nullptr; cur = strtok(nullptr, "&"))
69+
*query = '\0';
70+
mQuery = query + 1;
71+
72+
end = strchr(const_cast<char *>(mQuery), '#');
73+
74+
if (end == nullptr)
75+
{
76+
end = const_cast<char *>(mEnd);
77+
}
78+
79+
if (end > mQuery && end[-1] != '&')
6980
{
70-
cur[-1] = '\0';
81+
memmove(end + 1, end, mEnd - end + 1);
82+
*end = '&';
83+
mEnd = end + 1;
84+
}
85+
else
86+
{
87+
mEnd = end;
7188
}
7289
}
7390
else
@@ -97,21 +114,40 @@ const char *Url::GetValue(const char *aName, const char *aLastValue) const
97114

98115
while (start < mEnd)
99116
{
100-
const char *last = nullptr;
101-
102-
if (!strncmp(aName, start, len))
117+
if (!strncmp(aName, start, len) && (start[len] == '=' || start[len] == '&' || start[len] == '\0'))
103118
{
119+
const char *delimiter = start + len;
120+
121+
while (delimiter < mEnd && *delimiter != '&' && *delimiter != '\0')
122+
{
123+
delimiter++;
124+
}
125+
126+
if (delimiter < mEnd && *delimiter == '&')
127+
{
128+
*const_cast<char *>(delimiter) = '\0';
129+
}
130+
104131
if (start[len] == '=')
105132
{
106-
ExitNow(rval = &start[len + 1]);
133+
rval = &start[len + 1];
107134
}
108-
else if (start[len] == '\0')
135+
else
109136
{
110-
ExitNow(rval = &start[len]);
137+
rval = &start[len];
111138
}
139+
break;
140+
}
141+
142+
while (start < mEnd && *start != '&' && *start != '\0')
143+
{
144+
start++;
145+
}
146+
147+
if (start < mEnd)
148+
{
149+
start++;
112150
}
113-
last = start;
114-
start = last + strlen(last) + 1;
115151
}
116152

117153
exit:
@@ -202,5 +238,35 @@ otError Url::ParseInt8(const char *aName, int8_t &aValue) const
202238
return error;
203239
}
204240

241+
otError Url::Validate(const char **aUnusedParam) const
242+
{
243+
otError error = OT_ERROR_NONE;
244+
const char *cur = mQuery;
245+
246+
while (cur < mEnd)
247+
{
248+
if (*cur == '&')
249+
{
250+
const char *p = cur;
251+
252+
while (p > mQuery && p[-1] != '\0')
253+
{
254+
p--;
255+
}
256+
257+
if (aUnusedParam != nullptr)
258+
{
259+
*aUnusedParam = p;
260+
}
261+
262+
ExitNow(error = OT_ERROR_INVALID_ARGS);
263+
}
264+
cur++;
265+
}
266+
267+
exit:
268+
return error;
269+
}
270+
205271
} // namespace Url
206272
} // namespace ot

src/lib/url/url.hpp

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -192,6 +192,16 @@ class Url : public otUrl
192192
* @retval OT_ERROR_INVALID_ARGS The parameter value was not contain valid number (e.g., value out of range).
193193
*/
194194
otError ParseInt8(const char *aName, int8_t &aValue) const;
195+
196+
/**
197+
* Checks if all parameters are used.
198+
*
199+
* @param[out] aUnusedParam A pointer to a string to store the name of the first unused parameter.
200+
*
201+
* @retval OT_ERROR_NONE All parameters were used.
202+
* @retval OT_ERROR_INVALID_ARGS Some parameters were not used.
203+
*/
204+
otError Validate(const char **aUnusedParam = nullptr) const;
195205
};
196206

197207
} // namespace Url

src/posix/platform/hdlc_interface.cpp

Lines changed: 23 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -685,9 +685,29 @@ void HdlcInterface::CloseFile(void)
685685
#if OPENTHREAD_POSIX_CONFIG_RCP_PTY_ENABLE
686686
int HdlcInterface::ForkPty(const Url::Url &aRadioUrl)
687687
{
688-
int fd = -1;
689-
int pid = -1;
690-
int rval = -1;
688+
int fd = -1;
689+
int pid = -1;
690+
int rval = -1;
691+
constexpr int kMaxArguments = 32;
692+
char *argv[kMaxArguments + 1];
693+
size_t index = 0;
694+
695+
argv[index++] = const_cast<char *>(aRadioUrl.GetPath());
696+
697+
for (const char *arg = nullptr;
698+
index < OT_ARRAY_LENGTH(argv) && (arg = aRadioUrl.GetValue("forkpty-arg", arg)) != nullptr;
699+
argv[index++] = const_cast<char *>(arg))
700+
{
701+
}
702+
703+
if (index < OT_ARRAY_LENGTH(argv))
704+
{
705+
argv[index] = nullptr;
706+
}
707+
else
708+
{
709+
DieNowWithMessage("Too many arguments!", OT_EXIT_INVALID_ARGUMENTS);
710+
}
691711

692712
{
693713
struct termios tios;
@@ -701,27 +721,6 @@ int HdlcInterface::ForkPty(const Url::Url &aRadioUrl)
701721

702722
if (0 == pid)
703723
{
704-
constexpr int kMaxArguments = 32;
705-
char *argv[kMaxArguments + 1];
706-
size_t index = 0;
707-
708-
argv[index++] = const_cast<char *>(aRadioUrl.GetPath());
709-
710-
for (const char *arg = nullptr;
711-
index < OT_ARRAY_LENGTH(argv) && (arg = aRadioUrl.GetValue("forkpty-arg", arg)) != nullptr;
712-
argv[index++] = const_cast<char *>(arg))
713-
{
714-
}
715-
716-
if (index < OT_ARRAY_LENGTH(argv))
717-
{
718-
argv[index] = nullptr;
719-
}
720-
else
721-
{
722-
DieNowWithMessage("Too many arguments!", OT_EXIT_INVALID_ARGUMENTS);
723-
}
724-
725724
VerifyOrDie((rval = execvp(argv[0], argv)) != -1, OT_EXIT_ERROR_ERRNO);
726725
}
727726
else

src/posix/platform/radio.cpp

Lines changed: 26 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -63,8 +63,7 @@ extern "C" void platformRadioInit(const char *aUrl) { sRadio.Init(aUrl); }
6363
const char Radio::kLogModuleName[] = "Radio";
6464

6565
Radio::Radio(void)
66-
: mRadioUrl(nullptr)
67-
, mRadioSpinel()
66+
: mRadioSpinel()
6867
#if OPENTHREAD_POSIX_CONFIG_RCP_CAPS_DIAG_ENABLE
6968
, mRcpCapsDiag(mRadioSpinel)
7069
#endif
@@ -80,17 +79,19 @@ OT_TOOL_WEAK void platformCoprocessorResetFailed(void *) {}
8079

8180
void Radio::Init(const char *aUrl)
8281
{
83-
bool resetRadio;
84-
bool skipCompatibilityCheck;
82+
bool resetRadio;
83+
bool skipCompatibilityCheck;
84+
RadioUrl &radioUrl = SpinelManager::GetSpinelManager().GetRadioUrl();
85+
8586
#if OPENTHREAD_CONFIG_THREAD_VERSION >= OT_THREAD_VERSION_1_2 && OPENTHREAD_CONFIG_MAC_CSL_TRANSMITTER_ENABLE
8687
bool aEnableRcpTimeSync = true;
8788
#else
8889
bool aEnableRcpTimeSync = false;
8990
#endif
9091
struct ot::Spinel::RadioSpinelCallbacks callbacks;
9192

92-
mRadioUrl.Init(aUrl);
93-
VerifyOrDie(mRadioUrl.GetPath() != nullptr, OT_EXIT_INVALID_ARGUMENTS);
93+
VerifyOrDie(radioUrl.GetPath() != nullptr, OT_EXIT_INVALID_ARGUMENTS);
94+
OT_UNUSED_VARIABLE(aUrl);
9495

9596
memset(&callbacks, 0, sizeof(callbacks));
9697
callbacks.mEnergyScanDone = otPlatRadioEnergyScanDone;
@@ -105,8 +106,8 @@ void Radio::Init(const char *aUrl)
105106
mTmpStorage.Init();
106107
#endif
107108

108-
resetRadio = !mRadioUrl.HasParam("no-reset");
109-
skipCompatibilityCheck = mRadioUrl.HasParam("skip-rcp-compatibility-check");
109+
resetRadio = !radioUrl.HasParam("no-reset");
110+
skipCompatibilityCheck = radioUrl.HasParam("skip-rcp-compatibility-check");
110111

111112
#if OPENTHREAD_SPINEL_CONFIG_COPROCESSOR_RESET_FAILURE_CALLBACK_ENABLE
112113
GetSpinelDriver().SetCoprocessorResetFailureCallback(platformCoprocessorResetFailed, this);
@@ -116,7 +117,7 @@ void Radio::Init(const char *aUrl)
116117
mRadioSpinel.Init(skipCompatibilityCheck, resetRadio, &GetSpinelDriver(),
117118
(skipCompatibilityCheck ? 0 : kRequiredRadioCaps), aEnableRcpTimeSync);
118119

119-
ProcessRadioUrl(mRadioUrl);
120+
ProcessRadioUrl(radioUrl);
120121
}
121122

122123
void Radio::Deinit(void)
@@ -201,17 +202,21 @@ void Radio::ProcessMaxPowerTable(const RadioUrl &aRadioUrl)
201202

202203
#if OPENTHREAD_POSIX_CONFIG_MAX_POWER_TABLE_ENABLE
203204
otError error;
204-
constexpr int8_t kPowerDefault = 30; // Default power 1 watt (30 dBm).
205-
const char *str = nullptr;
206-
char *pSave = nullptr;
207-
uint8_t channel = ot::Radio::kChannelMin;
208-
int8_t power = kPowerDefault;
209-
const char *maxPowerTable;
205+
constexpr int8_t kPowerDefault = 30; // Default power 1 watt (30 dBm).
206+
char *str = nullptr;
207+
char *pSave = nullptr;
208+
uint8_t channel = ot::Radio::kChannelMin;
209+
int8_t power = kPowerDefault;
210+
const char *maxPowerTable = nullptr;
211+
char *maxPowerTableCopy = nullptr;
210212

211213
VerifyOrExit((maxPowerTable = aRadioUrl.GetValue("max-power-table")) != nullptr);
212214

213-
for (str = strtok_r(const_cast<char *>(maxPowerTable), ",", &pSave);
214-
str != nullptr && channel <= ot::Radio::kChannelMax; str = strtok_r(nullptr, ",", &pSave))
215+
maxPowerTableCopy = strdup(maxPowerTable);
216+
VerifyOrDie(maxPowerTableCopy != nullptr, OT_EXIT_FAILURE);
217+
218+
for (str = strtok_r(maxPowerTableCopy, ",", &pSave); str != nullptr && channel <= ot::Radio::kChannelMax;
219+
str = strtok_r(nullptr, ",", &pSave))
215220
{
216221
power = static_cast<int8_t>(strtol(str, nullptr, 0));
217222
error = mRadioSpinel.SetChannelMaxTransmitPower(channel, power);
@@ -240,6 +245,10 @@ void Radio::ProcessMaxPowerTable(const RadioUrl &aRadioUrl)
240245
VerifyOrDie(str == nullptr, OT_EXIT_INVALID_ARGUMENTS);
241246

242247
exit:
248+
if (maxPowerTableCopy != nullptr)
249+
{
250+
free(maxPowerTableCopy);
251+
}
243252
return;
244253
#endif // OPENTHREAD_POSIX_CONFIG_MAX_POWER_TABLE_ENABLE
245254
}

src/posix/platform/radio.hpp

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -140,7 +140,6 @@ class Radio : public Logger<Radio>
140140
#endif
141141
OT_RADIO_CAPS_ACK_TIMEOUT | OT_RADIO_CAPS_TRANSMIT_RETRIES | OT_RADIO_CAPS_CSMA_BACKOFF;
142142

143-
RadioUrl mRadioUrl;
144143
#if OPENTHREAD_SPINEL_CONFIG_VENDOR_HOOK_ENABLE
145144
Spinel::VendorRadioSpinel mRadioSpinel;
146145
#else

src/posix/platform/radio_url.cpp

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -149,8 +149,10 @@ void RadioUrl::Init(const char *aUrl)
149149
{
150150
if (aUrl != nullptr)
151151
{
152-
VerifyOrDie(strnlen(aUrl, sizeof(mUrl)) < sizeof(mUrl), OT_EXIT_INVALID_ARGUMENTS);
153-
strncpy(mUrl, aUrl, sizeof(mUrl) - 1);
152+
size_t len = strlen(aUrl);
153+
154+
VerifyOrDie(len + 1 < sizeof(mUrl), OT_EXIT_INVALID_ARGUMENTS);
155+
strcpy(mUrl, aUrl);
154156
SuccessOrDie(Url::Url::Init(mUrl));
155157
}
156158
}

src/posix/platform/spinel_manager.hpp

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -91,6 +91,13 @@ class SpinelManager
9191
return *mSpinelInterface;
9292
}
9393

94+
/**
95+
* Returns the radio URL.
96+
*
97+
* @returns The radio URL.
98+
*/
99+
RadioUrl &GetRadioUrl(void) { return mUrl; }
100+
94101
private:
95102
#if OPENTHREAD_POSIX_VIRTUAL_TIME
96103
void VirtualTimeInit(void);

src/posix/platform/system.cpp

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,7 @@
5959
#include "posix/platform/mdns_socket.hpp"
6060
#include "posix/platform/radio_url.hpp"
6161
#include "posix/platform/spinel_driver_getter.hpp"
62+
#include "posix/platform/spinel_manager.hpp"
6263
#include "posix/platform/udp.hpp"
6364

6465
otInstance *gInstance = nullptr;
@@ -275,6 +276,16 @@ otInstance *otSysInit(otPlatformConfig *aPlatformConfig)
275276

276277
platformInit(aPlatformConfig);
277278

279+
{
280+
const char *unusedParam = nullptr;
281+
282+
if (ot::Posix::SpinelManager::GetSpinelManager().GetRadioUrl().Validate(&unusedParam) != OT_ERROR_NONE)
283+
{
284+
otLogCritPlat("Radio URL contains unused parameter: \"%s\"", unusedParam);
285+
DieNow(OT_EXIT_INVALID_ARGUMENTS);
286+
}
287+
}
288+
278289
gDryRun = aPlatformConfig->mDryRun;
279290
if (sCoprocessorType == OT_COPROCESSOR_RCP)
280291
{

src/posix/platform/trel.cpp

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -658,6 +658,15 @@ void platformTrelInit(const char *aTrelUrl)
658658
ot::Posix::RadioUrl url(aTrelUrl);
659659

660660
otSysTrelInit(url.GetPath());
661+
{
662+
const char *unusedParam = nullptr;
663+
664+
if (url.Validate(&unusedParam) != OT_ERROR_NONE)
665+
{
666+
otLogCritPlat("TREL radio URL contains unused parameter: \"%s\"", unusedParam);
667+
DieNow(OT_EXIT_INVALID_ARGUMENTS);
668+
}
669+
}
661670
}
662671
}
663672

0 commit comments

Comments
 (0)