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 Underflow vulnarability assertion check fix for RFC1751.cpp #5127

Open
wants to merge 4 commits into
base: develop
Choose a base branch
from

Conversation

nathanogaga118
Copy link

an assertion check fix has been updated with the code

https://github.com/nathanogaga118/rippled/blob/develop/src/libxrpl/crypto/RFC1751.cpp

Key Changes and fixes:

Assertions Added:

In btoe: assert(strData.size() >= 8 && "strData must be at least 8 bytes long");

In getEnglishFromKey: assert(strKey.size() >= 16 && "strKey must be at least 16 bytes long");

These assertions ensure that the preconditions for the btoe function are met, preventing potential buffer underflow issues.

Key Points:

Error Handling: The btoe function now checks if strData is at least 8 bytes long, throwing an exception if not.

Standardization: The standard function converts input words to a standardized format.

Word Search: The wsrch function performs a binary search on the dictionary.

Encoding and Decoding: Functions like etob and getKeyFromEnglish handle the conversion between binary data and English words.

This complete code now includes necessary checks to prevent buffer underflow and ensures safer handling of input data.
Key Changes:

Logging Improvements:

Changed log messages to avoid directly logging sensitive data like IP addresses and ports.

Used more generic messages to indicate configuration errors without exposing sensitive details.

Error Handling:
Used Rethrow() to propagate exceptions after logging a generic error message.

Security Considerations:

Ensure logs are securely stored and access is restricted to authorized personnel to prevent unauthorized access to potentially sensitive configuration details.
assertion added
@Bronek
Copy link
Collaborator

Bronek commented Sep 13, 2024

Please kindly re-format this PR, as per to https://github.com/XRPLF/rippled/blob/develop/CONTRIBUTING.md#formatting Note, we are using old clang-format-10, but it's trivial to setup with the help of containerised ubuntu:20.04 (https://packages.ubuntu.com/focal/clang-format-10 )

@Bronek
Copy link
Collaborator

Bronek commented Sep 13, 2024

Also, this PR seems to be removing existing comments from the codebase in a rather arbitrary manner, please do not do that without a good reason.

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