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

Fix potential char overflow bug #515

Merged
merged 2 commits into from
Apr 26, 2023
Merged

Conversation

hellkite500
Copy link
Member

@hellkite500 hellkite500 commented Apr 24, 2023

A potential overflow flow bug exists in the read_file_line_counts function. Depending on system, compiler, and hardware, this can lead to a infinite loop.

This bug has also been patched in these repositories as discovered.

It may pop again in the future in different modules if the original bug was copy/pasted more. If you think you are stuck initializing a module that shouldn't take forever...grep for fgetc and check if it is reading into a char, if so, it should be an int.

Changes

  • Change the i/o buffer type for character reading to int from char to avoid overflowing a possible unsigned char type.

Testing

  1. Tested through ngen unit tests

Checklist

  • PR has an informative and human-readable title
  • Changes are limited to a single goal (no scope creep)
  • Code can be automatically merged (no conflicts)
  • Code follows project standards (link if applicable)
  • Passes all existing automated tests
  • Any change in functionality is tested
  • New functions are documented (with a description, list of inputs, and expected output)
  • Placeholder code is flagged / future todos are captured in comments
  • Project documentation has been updated (including the "Unreleased" section of the CHANGELOG)
  • Reviewers requested with the Reviewers tool ➡️

Testing checklist

Target Environment support

  • Linux (x86 and arm)
  • MacOS (x86 and arm)

Copy link
Contributor

@donaldwj donaldwj left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, I think this is on the list of classic Fortran errors.

@hellkite500 hellkite500 merged commit 8d41dd3 into NOAA-OWP:master Apr 26, 2023
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