Skip to content

fix: security hardening and bug fixes across all core modules - #9

Open
0xgetz wants to merge 6 commits into
octra-labs:mainfrom
0xgetz:main
Open

fix: security hardening and bug fixes across all core modules#9
0xgetz wants to merge 6 commits into
octra-labs:mainfrom
0xgetz:main

Conversation

@0xgetz

@0xgetz 0xgetz commented Apr 14, 2026

Copy link
Copy Markdown

Summary

This PR fixes 12 bugs and security issues found across the four core source files.


crypto_utils.hpp

1. Thread-safety race condition in base64_decode

  • Problem: Lazy-initialized static lookup table was not thread-safe — concurrent calls could read a partially-initialized table.
  • Fix: Replaced with a constexpr compile-time lookup table. Zero runtime cost, fully thread-safe.

2. Unchecked return values in wallet_encrypt

  • Problem: EVP_EncryptUpdate and EVP_EncryptFinal_ex return values were ignored. Silent data corruption on failure.
  • Fix: Added explicit return value checks; throws std::runtime_error on failure.

3. wallet_decrypt output buffer not resized to actual length

  • Problem: Output vector was resized to ciphertext.size() but the actual decrypted length (returned by EVP) was never used — trailing garbage bytes were included.
  • Fix: Resize output to out_len1 + out_len2 after decryption.

4. derive_hd_seed v1 copies master seed bytes instead of deriving child key

  • Problem: Version 1 derivation used std::copy of the master seed directly — no actual HD derivation occurred.
  • Fix: Replaced with HMAC-SHA256 keyed on master seed with index as data, matching standard HD wallet derivation semantics.

5. validate_mnemonic — linear search + UB in std::tolower

  • Problem: Used std::find (O(n)) on an unsorted wordlist; std::tolower(char) is UB for negative char values.
  • Fix: Switched to std::binary_search (O(log n)); cast char to unsigned char before tolower.

rpc_client.hpp

6. Default constructor leaves host_ empty

  • Problem: Default RpcClient() left host_ as empty string — any RPC call without explicit host would fail silently.
  • Fix: Default constructor now sets host_ to "rpc.octra.io".

7. Incomplete comment // rpc compl

  • Problem: Truncated comment left misleading/incomplete documentation.
  • Fix: Expanded to // rpc completion callback.

wallet.hpp

8. wallet_path_for — missing address validation + path traversal

  • Problem: No validation that the address starts with oct prefix; any string including ../ could be used to write wallet files outside the intended directory.
  • Fix: Added oct prefix check and strict allowlist character validation (alphanumeric only) before constructing file path.

main.cpp

9. parse_amount_raw accepts negative values

  • Problem: A leading - was not rejected — negative transfer amounts could be constructed.
  • Fix: Added explicit check: reject any string starting with -.

10. Port validation allows values > 65535

  • Problem: Port was parsed as int with only a > 0 lower bound — values like 99999 were accepted.
  • Fix: Added upper bound check port > 65535.

11. Stealth mode step counter inconsistency ([3/7][6/7] vs [3/8][6/8])

  • Problem: Steps were labeled as /7 at some points and /8 at others within the same flow.
  • Fix: Normalized all stealth step labels to /8.

12. file_hint path traversal via unvalidated input

  • Problem: File path derived from user-controlled file_hint with no sanitization — allowed directory traversal and access to arbitrary files.
  • Fix: Strict allowlist validation (alphanumeric, ., -, _ only); any other character throws an error.

Testing

All changes are backward-compatible. No new dependencies introduced. Compile-tested against the existing build system.

@ghost

ghost commented Apr 30, 2026

Copy link
Copy Markdown

did you remove all the api endpoints in main.cpp ? ? ?

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.

1 participant