search

rdfhdt / hdt-cpp

HDT C++ Library and Tools
117 stars 65 forks source link

Fix possible buffer overflow #275

Closed kamahen closed 3 months ago

kamahen commented 10 months ago

This PR should be applied to branch develop.

These changes are because the compiler pointed out mis-use of strncmp() and memcpy(). I think that I've fixed the problems, but it's easy to make an off-by-one error in such code, so please review.

(A better way of fixing this would be to use std::string or std::basic_string<unsigned char>)

kamahen commented 10 months ago

The files to check are:

libhdt/src/libdcs/CSD_FMIndex.cpp libhdt/src/libdcs/CSD_HTFC.cpp libhdt/src/libdcs/CSD_PFC.cpp libhdt/src/triples/TripleListDisk.cpp

The other files are straightforward responses to compiler warnings.

kamahen commented 10 months ago

The code now compiles cleanly using gcc-13.2 with -Wall -Wextra. There were some additional problems with the use of strncpy() that I fixed. (And, in an earlier commit, I hadn't correctly fixed a call to strncpy() -- I think it's correct now). None of these compiler warnings resulted in any failures of the test cases, so I have no easy way of verifying that I've fixed anything, nor that I've fixed things correctly.

kamahen commented 8 months ago

This is ready to merge into branch develop. (using this? https://github.com/rdfhdt/hdt-cpp/compare/develop...JanWielemaker:hdt-cpp:develop-buffer)

Or I can do the merge on my local copy and push to branch develop ... it's up to you. (A 2nd pair of eyes, looking at my changes, is always appreciated)

Once that is done, I'll create a PR for the hdt pack that references this.

kamahen commented 4 months ago

I reviewed my changes and realize that there's a mistake in the calls to strncpy(). I will fix this and push a new commit.

kamahen commented 4 months ago

I've looked at the 5 conflicts, and in each case, the first one ("develop-buffer") should be chosen (except, you might choose to ignore my "TODO").

I suspect that there are other potential buffer overflows in the code -- it would have been better if C++ std::vector or std::string had been used instead of C-style char* (especially with null-termination), but it appears that retro-fixing this is a non-trivial job, and it also appears that this project is no longer being maintained.

kamahen commented 3 months ago

This PR has been superseded by https://github.com/rdfhdt/hdt-cpp/pull/283 (same changes but no bogus merge conflicts).