Skip to content

Fix integer overflow on 16-bit MCUs in timeout calculations #2

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.

Sign up for GitHub

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

Draft
Copilot wants to merge 5 commits into
base: master
Choose a base branch
from
Draft

Fix integer overflow on 16-bit MCUs in timeout calculations #2

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
f7d53a4
Initial plan for issue
Copilot Jun 9, 2025
ec82b76
Fix integer overflow for 90*1000 on 16-bit MCUs
Copilot Jun 9, 2025
e169fc1
Update tests to use consistent unsigned arithmetic
Copilot Jun 9, 2025
07abe7d
Ensure consistent unsigned arithmetic for all timeout multiplications
Copilot Jun 9, 2025
a61c3b8
Fix 16-bit integer overflow by using UL suffix for all timeout multip…
Copilot Jun 9, 2025
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
14 changes: 7 additions & 7 deletions n_request.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@ void _noteSuspendTransactionDebug(void)
*/
NOTE_C_STATIC uint32_t _noteTransaction_calculateTimeoutMs(J *req, bool isReq)
{
uint32_t result = (CARD_INTER_TRANSACTION_TIMEOUT_SEC * 1000);
uint32_t result = (CARD_INTER_TRANSACTION_TIMEOUT_SEC * 1000UL);

// Interrogate the request
if (JContainsString(req, (isReq ? "req" : "cmd"), "note.add")) {
Expand All @@ -123,7 +123,7 @@ NOTE_C_STATIC uint32_t _noteTransaction_calculateTimeoutMs(J *req, bool isReq)
result = JGetInt(req, "milliseconds");
} else if (JIsPresent(req, "seconds")) {
NOTE_C_LOG_DEBUG("Using `seconds` parameter value for timeout.");
result = (JGetInt(req, "seconds") * 1000);
result = (JGetInt(req, "seconds") * 1000UL);
}
} else if (JContainsString(req, (isReq ? "req" : "cmd"), "web.")) {
NOTE_C_LOG_DEBUG("web.* request received.");
Expand All @@ -133,11 +133,11 @@ NOTE_C_STATIC uint32_t _noteTransaction_calculateTimeoutMs(J *req, bool isReq)
result = JGetInt(req, "milliseconds");
} else if (JIsPresent(req, "seconds")) {
NOTE_C_LOG_DEBUG("Using `seconds` parameter value for timeout.");
result = (JGetInt(req, "seconds") * 1000);
result = (JGetInt(req, "seconds") * 1000UL);
} else {
NOTE_C_LOG_DEBUG("No `milliseconds` or `seconds` parameter "
"provided. Defaulting to 90-second timeout.");
result = (90 * 1000);
result = (90UL * 1000UL);
}
}

Expand Down Expand Up @@ -319,7 +319,7 @@ J *NoteRequestResponseWithRetry(J *req, uint32_t timeoutSeconds)

// Calculate expiry time in milliseconds
uint32_t startMs = _GetMs();
uint32_t timeoutMs = timeoutSeconds * 1000;
uint32_t timeoutMs = timeoutSeconds * 1000UL;

while(true) {
// Execute the transaction
Expand Down Expand Up @@ -378,7 +378,7 @@ J *NoteRequestResponseWithRetry(J *req, uint32_t timeoutSeconds)
*/
char * NoteRequestResponseJSON(const char *reqJSON)
{
const uint32_t transactionTimeoutMs = (CARD_INTER_TRANSACTION_TIMEOUT_SEC * 1000);
const uint32_t transactionTimeoutMs = (CARD_INTER_TRANSACTION_TIMEOUT_SEC * 1000UL);
char *rspJSON = NULL;
char *allocatedJSON = NULL; // required to free the string if it is not newline-terminated
bool isCmdPipeline = false;
Expand Down Expand Up @@ -579,7 +579,7 @@ J *_noteTransactionShouldLock(J *req, bool lockNotecard)
const uint32_t id = JGetInt(req, "id");

// Ensure the Notecard is ready
if (!_TransactionStart(CARD_INTER_TRANSACTION_TIMEOUT_SEC * 1000)) {
if (!_TransactionStart(CARD_INTER_TRANSACTION_TIMEOUT_SEC * 1000UL)) {
_Free(json);
const char *errStr = ERRSTR("Notecard not ready (CTX/RTX) {io}", c_ioerr);
if (cmdFound) {
Expand Down
4 changes: 2 additions & 2 deletions test/src/_noteTransaction_calculateTimeoutMs_test.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -377,7 +377,7 @@ SCENARIO("_noteTransaction_calculateTimeoutMs")
J *resp = NoteTransaction(req);

THEN("The timeout value is set to 90 seconds") {
CHECK(_noteJSONTransaction_fake.arg3_val == (90 * 1000));
CHECK(_noteJSONTransaction_fake.arg3_val == (90UL * 1000UL));
}

JDelete(resp);
Expand All @@ -394,7 +394,7 @@ SCENARIO("_noteTransaction_calculateTimeoutMs")
J *resp = NoteTransaction(req);

THEN("The timeout value is set to 90 seconds") {
CHECK(_noteJSONTransaction_fake.arg3_val == (90 * 1000));
CHECK(_noteJSONTransaction_fake.arg3_val == (90UL * 1000UL));
}

JDelete(resp);
Expand Down