Skip to content

Migrate raw pointers to unique_ptr and fix memory/stability issues#38

Open
crayy8 wants to merge 8 commits intoSleuthKitLabs:develop-4.14from
crayy8:rejistry_updates
Open

Migrate raw pointers to unique_ptr and fix memory/stability issues#38
crayy8 wants to merge 8 commits intoSleuthKitLabs:develop-4.14from
crayy8:rejistry_updates

Conversation

@crayy8
Copy link
Copy Markdown
Member

@crayy8 crayy8 commented Mar 24, 2026

  • Replace raw pointer ownership with unique_ptr throughout the library: RegistryByteBuffer, ValueData, RegistryKey, RegistryValue, RegistryHiveFile, RegistryHiveBuffer, Cell, HBIN, REGFHeader, NKRecord, VKRecord, ValueListRecord
  • Remove AutoCellPtrList and AutoHBINPtrList RAII wrappers now that unique_ptr handles lifetime automatically
  • Add virtual destructor to BinaryBlock to prevent UB when deleting through base class pointer
  • Fix integer overflow in ByteBuffer::read() bounds check via uint64_t cast
  • Fix dangling pointer in RejistryException::what() by caching result in mutable member
  • Fix RegistryHiveFile::getErrorMessage() to handle FormatMessageA failure
  • Fix REGFHeader::getHBINs() to std::move unique_ptr into vector
  • Refactor Rejistry test program to exercise both RegistryHiveFile and RegistryHiveBuffer through shared processRegistryHive() function

- Replace raw pointer ownership with unique_ptr throughout the library:
  RegistryByteBuffer, ValueData, RegistryKey, RegistryValue,
  RegistryHiveFile, RegistryHiveBuffer, Cell, HBIN, REGFHeader,
  NKRecord, VKRecord, ValueListRecord
- Remove AutoCellPtrList and AutoHBINPtrList RAII wrappers now that
  unique_ptr handles lifetime automatically
- Add virtual destructor to BinaryBlock to prevent UB when deleting
  through base class pointer
- Fix integer overflow in ByteBuffer::read() bounds check via uint64_t cast
- Fix dangling pointer in RejistryException::what() by caching result
  in mutable member
- Fix RegistryHiveFile::getErrorMessage() to handle FormatMessageA failure
- Fix REGFHeader::getHBINs() to std::move unique_ptr into vector
- Refactor Rejistry test program to exercise both RegistryHiveFile and
  RegistryHiveBuffer through shared processRegistryHive() function
crayy8 added 4 commits March 24, 2026 11:21
- Replace raw pointer typedefs with unique_ptr equivalents across
  NKRecord, VKRecord, HBIN, SubkeyListRecord, ValueListRecord,
  RegistryKey, RegistryValue, ValueData, and RegistryHive
- Remove AutoNKRecordPtrList and AutoVKRecordPtrList RAII wrappers
- Return unique_ptr from getRoot(), getSubkeyList(), getValueList(),
  getValue(), getSubkeys(), and getValues() throughout the hierarchy
- Add bounds check in HBIN::getCells() to prevent over-reading hbin
- Delete copy assignment operators on Buffer and Record base classes
- Replace NULL with nullptr in HBIN and RegistryByteBuffer
crayy8 and others added 2 commits April 9, 2026 09:45
- Remove undocumented MAX_NAME_LENGTH check on class names (no spec limit exists)
- Fix class name UTF-16LE decoding via reinterpret_cast
- Fix inline QWORD/FILETIME data handling when high bit signals inline storage
- Add safeLength clamping for cell-stored QWORD/FILETIME values
- Use getSubkeyCount() instead of getSubkeyList()->getListLength()
- Remove dead null checks after unique_ptr construction
- Remove redundant break in HBIN cell iteration
- Remove duplicate processRegistryBuffer pass from test driver
- Add comprehensive test hive generator (33 scenarios)

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Replace raw pointer usage and std::auto_ptr with std::unique_ptr
throughout RegParser.cpp, RegKey.cpp, and RegVal.cpp. Use .release(),
.get(), auto, and range-for loops to adapt to the new ownership
semantics. Remove manual delete calls that are now handled by
unique_ptr destructors.
@crayy8 crayy8 requested a review from APriestman April 13, 2026 13:21
@crayy8
Copy link
Copy Markdown
Member Author

crayy8 commented Apr 13, 2026

This now has all of rejistry++ using unqiue pointers so no manual memory management is needed.
This also has the necessary updates to tsk. The changes to tsk were minimal and converts all unqiue pointers back to raw pointers to keep changes there minimal and b/c that tsk code does not seem to be important from what I understand from Ann. We can also go back and update tsk if we decide we care about logical imager.

…mprovements

- EmptySubkeyList: override getListLength() (returns 0) and shadow
  getMagic() (returns "") to prevent reading garbage from REGF header
  when subkey count is zero
- SubkeyListRecord: make getListLength() virtual to enable override
- Rejistry.cpp test driver: add SEH wrapper to catch stack overflow
  and other structured exceptions with diagnostic output to both
  stdout and stderr; add MAX_RECURSION_DEPTH=1000 safety check;
  cap named lookups at 10 per key to avoid O(n^2) on large subkey
  lists; add hive summary stats (HBIN/cell/key/value counts, type
  breakdowns, extremes); condense HBIN/cell output to one line each
- Rejistry.vcxproj: increase stack reserve to 4MB across all configs
  to handle deeply nested hives (500+ levels)
@crayy8
Copy link
Copy Markdown
Member Author

crayy8 commented Apr 14, 2026

@APriestman if you care there is https://basistech.atlassian.net/browse/CT-11222 that has the test scripts I created to test all of this plus the test hives and results of the diff against ct_3.16 tsk branch. Everything looks good. No regressions but a bunch of fixes. It was crashing on 15+ of the hives and some data was not reported in certain scenarios and now we have no crashes

I'm not going to remove do not merge label until the CT PR is ready but this can be reviewed when you have time.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants