search

u-blox / ubxlib

Portable C libraries which provide APIs to build applications with u-blox products and services. Delivered as add-on to existing microcontroller and RTOS SDKs.
Apache License 2.0
303 stars 92 forks source link

AT Client Character Wait Fails Once Ticks Wrap Beyond INT32_MAX #208

Closed warasilapm closed 5 months ago

warasilapm commented 7 months ago

Problem Description

The AT client private function uATClientWaitCharacter() does not correctly handle ticks after wrap around.

https://github.com/u-blox/ubxlib/blob/9694ef9d70e6e7d90562174e8c4d52555d8730da/common/at_client/src/u_at_client.c#L3927-L3976

This can lead to two failure cases:

  1. If the starting tick is in the range of (INT32_MAX - timeoutMs, INT32_MAX], the check for wrap around causes the timeout to immediately occur.
  2. However, if the starting tick is in the range [INT32_MIN, 0], this attempt to check for wrap around instead causes the timeout to only occur when the tick passes timeoutMs again - which could be as long as -INT32_MIN + timeoutMs ticks, or about 25 days.

If the AT server operates correctly, this should not strictly cause a problem. However, a failure during the interval between INT32_MIN and zero which does not provide sufficient characters could take an unexpectedly long time to time out and fail. Additionally, I believe it is possible for the first case to cause a parsing failure in the AT client which would result in a short response length being provided during a subsequent command attempt, triggering the second case.

Suggested Solution

Fix the buggy check to handle wrap around by calculating a difference instead of stop time. See the attached patch.

RobMeades commented 7 months ago

Again, thanks for spotting this, same conclusion/procedure as in #207 applies.

RobMeades commented 5 months ago

The fix for his issue is included in the rather larger fix of commit 79e808eb8be3095b4f94efcffba97515ba1870bd.