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

Buffer overflow in unit tests when using default config #66

Closed
2 tasks done
jphickey opened this issue Jan 16, 2023 · 0 comments · Fixed by #67
Closed
2 tasks done

Buffer overflow in unit tests when using default config #66

jphickey opened this issue Jan 16, 2023 · 0 comments · Fixed by #67
Assignees
Milestone

Comments

@jphickey
Copy link
Contributor

Checklist (Please check before submitting)

  • I reviewed the Contributing Guide.
  • I performed a cursory search to see if the bug report is relevant, not redundant, nor in conflict with other tickets.

Describe the bug
The unit test code sets the EventText member with a call to strncpy and a hardcoded size here:

strncpy(LC_OperData.ADTPtr[APNumber].EventText, "Event Message", 50);

However in the default platform config the size is only 32:

#define LC_MAX_ACTION_TEXT 32

To Reproduce
Build and run using default/out-of-box config.

Expected behavior
Example configuration should not trigger buffer overflow

Additional context
Consider using sizeof() operator here, to adapt the strncpy call to the real size of the target buffer.

Reporter Info
Joseph Hickey, Vantage Systems, Inc.

@jphickey jphickey added the bug label Jan 16, 2023
@jphickey jphickey self-assigned this Jan 16, 2023
jphickey added a commit to jphickey/LC that referenced this issue Jan 16, 2023
The default size of the "EventText" string is 32, but the unit test had
hard coded string sizes of 50 chars, which will overflow the buffer.

Use "sizeof" operator to adjust to the actual size of the destination to
correct the issue.
dzbaker added a commit that referenced this issue Jan 19, 2023
Fix #66, correct buffer overflows in UT
@dzbaker dzbaker added this to the Draco milestone Feb 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants