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

Vulnerability: Comparison of narrow type with wide type in loop condition #1285

Open
FeedehC opened this issue Feb 28, 2023 · 1 comment
Open
Labels

Comments

@FeedehC
Copy link

FeedehC commented Feb 28, 2023

I'm working on a project using ESPAsyncWebServer and decided to implement vulnerability scans with CodeQL in GitHub Actions. It also scans libraries.
It is detecting in the file /src/WebRequest.cpp in lines 538 and 548 there are comparisons between "i" of type uint8_t and "call to length" of wider type unsigned int.

CodeQL information:
In a loop condition, comparison of a value of a narrow type with a value of a wide type may result in unexpected behavior if the wider value is sufficiently large (or small). This is because the narrower value may overflow. This can lead to an infinite loop.
Recommendation: Change the types of the compared values so that the value on the narrower side of the comparison is at least as wide as the value it is being compared with.

Example:
In this example, bytes_received is compared against max_get in a while loop. However, bytes_received is an int16_t, and max_get is an int32_t. Because max_get is larger than INT16_MAX, the loop condition is always true, so the loop never terminates.

This problem is avoided in the 'GOOD' case because bytes_received2 is an int32_t, which is as wide as the type of max_get.

void main(int argc, char **argv) {
	  uint32_t big_num = INT32_MAX;
	  char buf[big_num];
	  int16_t bytes_received = 0;
	  int max_get = INT16_MAX + 1;
  
	  // BAD: 'bytes_received' is compared with a value of a wider type.
	  // 'bytes_received' overflows before  reaching 'max_get',
	  // causing an infinite loop
	  while (bytes_received < max_get)
		  bytes_received += get_from_input(buf, bytes_received);
	  }
  
	  uint32_t bytes_received = 0;
  
	  // GOOD: 'bytes_received2' has a type  at least as wide as 'max_get'
	  while (bytes_received < max_get) {
		  bytes_received += get_from_input(buf, bytes_received);
	  }
  
  }
  
  
  int getFromInput(char *buf, short pos) {
	  // write to buf
	  // ...
	  return 1;
  }

References:
Data type ranges
INT18-C. Evaluate integer expressions in a larger size before comparing or assigning to that size
Common Weakness Enumeration: CWE-190.
Common Weakness Enumeration: CWE-197.
Common Weakness Enumeration: CWE-835.

@stale
Copy link

stale bot commented May 21, 2023

[STALE_SET] This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 14 days if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label May 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

1 participant