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 ndpi_analyze.c at lines 101, 104, 107, 110 #1999

Merged
merged 5 commits into from
May 30, 2023

Conversation

headshog
Copy link
Contributor

Hi! We've been fuzzing nDPI with sydr-fuzz security predicates and numeric truncation error was found in ndpi_analyze.c on lines 101, 104, 107, 110.

In ndpi_data_add_value the structure struct ndpi_analyze_struct *s is used. The variable value has const u_int64_t type. In this function at lines 101, 104, 107, 110 numeric truncation error may occur, because the fields of the structure ndpi_analyze_struct (*values, min_val, max_val, sum_total) has u_int32_t type. So we suggest to change the type of these fields to u_int64_t in ndpi_typedefs.h. And I've also tried to find all cases where the pointer values is used and fixed the types of dependent variables and constant numbers.

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 sydr_080510740ca15f16acb0a2356e8b2c7925bc7e56_num_trunc_5.txt
    
    
  4. Output:

    ndpi_analyze.c:101:31: runtime error: implicit conversion from type 'u_int64_t' (aka 'unsigned long') of value 1056964596001 (64-bit, unsigned) to type 'u_int32_t' (aka 'unsigned int') changed the value to 402641185 (32-bit, unsigned)
    SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior ndpi_analyze.c:101:31 in 
    ndpi_analyze.c:107:16: runtime error: implicit conversion from type 'unsigned long' of value 1056964596001 (64-bit, unsigned) to type 'u_int32_t' (aka 'unsigned int') changed the value to 402641185 (32-bit, unsigned)
    SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior ndpi_analyze.c:107:16 in 
    ndpi_analyze.c:110:45: runtime error: implicit conversion from type 'u_int64_t' (aka 'unsigned long') of value 1056964596001 (64-bit, unsigned) to type 'u_int32_t' (aka 'unsigned int') changed the value to 402641185 (32-bit, unsigned)
    SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior ndpi_analyze.c:110:45
    ndpi_analyze.c:104:41: runtime error: implicit conversion from type 'u_int64_t' (aka 'unsigned long') of value 1073754425873 (64-bit, unsigned) to type 'u_int32_t' (aka 'unsigned int') changed the value to 12601873 (32-bit, unsigned)
    SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior ndpi_analyze.c:104:41
    

@sonarcloud
Copy link

sonarcloud bot commented May 30, 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

@headshog
Copy link
Contributor Author

@IvanNardi Hi! I fixed tests.

@IvanNardi IvanNardi merged commit a8d2eed into ntop:dev May 30, 2023
@IvanNardi
Copy link
Collaborator

@headshog, thank you for your contributions

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