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

Numeric truncation at rtcp.c:49 #2033

Merged
merged 1 commit into from
Jul 5, 2023
Merged

Conversation

headshog
Copy link
Contributor

@headshog headshog commented Jul 5, 2023

Hi! We've been fuzzing nDPI with sydr-fuzz security predicates and we found numeric truncation error in rtcp:49.

In function ndpi_search_rtcp on line 49 rtcp_section_len and offset variables has types u_int16_t. But due to integer promotion the right side of operator has int type, so the numeric truncation may occur. Variable rtcp_section_len is used after in operator offset += rtcp_section_len on line 55. Variable offset participates in the cycle condition offset + 3 < packet->payload_packet_len, what means that truncation error may affect the work of cycle. So we suggest to change the type u_int16_t of these variables to type u_int32_t.

Environment

How to reproduce this error

  1. Build docker container:

    sudo docker build -t oss-sydr-fuzz-ndpi .
    
    
  2. Run docker container:

    docker run --privileged --network host -v /etc/localtime:/etc/localtime:ro --rm -it -v $PWD:/fuzz oss-sydr-fuzz-ndpi /bin/bash
    
    
  3. Run on the following input:

    /nDPI/libfuzzer/fuzz_ndpi_reader_alloc_fail sydr_c5c5f727ee380f5fc47d7bb8c16543e084dcebfd_num_trunc_7_unsigned.txt
    
    
  4. Output:

    protocols/rtcp.c:49:26: runtime error: implicit conversion from type 'int' of value 99744 (32-bit, signed) to type 'u_int16_t' (aka 'unsigned short') changed the value to 34208 (16-bit, unsigned)
    SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior protocols/rtcp.c:49:26
    

@sonarcloud
Copy link

sonarcloud bot commented Jul 5, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

@IvanNardi IvanNardi merged commit d51f892 into ntop:dev Jul 5, 2023
33 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants