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

Segfault in lsbom #33

Open
cluck opened this issue Mar 12, 2023 · 3 comments
Open

Segfault in lsbom #33

cluck opened this issue Mar 12, 2023 · 3 comments

Comments

@cluck
Copy link
Contributor

cluck commented Mar 12, 2023

Thank you for this great utility!

Unfortunately, commit 704739c introduced a segfault in lsbom, which is triggered verifying any Bom. Even the provided docker image fails to build.

I resolved the issue as follows:

diff --git a/src/lsbom.cpp b/src/lsbom.cpp
index 22d51e7..94fbb90 100644
--- a/src/lsbom.cpp
+++ b/src/lsbom.cpp
@@ -268,7 +268,7 @@ int main(int argc, char *argv[]) {

       DEBUG(2, "BOMVar 0x" << hex << ntohl(var->index) << ' ' << name << ':');

-      if (strstr(name.c_str(),"Paths") == 0) {
+      if (name.rfind("Paths", 0) == 0) {
         BOMPaths *paths = (BOMPaths *)lookup(tree->child);

         typedef map<uint32_t, string> filenames_t;

Passing c_str() pointers to standard library string functions is problematic; the C++ reference says the pointer obtained from c_str() may be invalidated by passing a non-const reference to the string to any standard library function

@hogliux
Copy link
Owner

hogliux commented Mar 16, 2023

Why are you using rfind? Wouldn't find be beter?

@cluck
Copy link
Contributor Author

cluck commented Oct 26, 2024

Uh, sorry for never coming back here.

I inferred from the original change that the essence still was to check for name to contain the exact string "Paths" or to start with "Paths".

-      if (name == "Paths") {
+      if (strstr(name.c_str(),"Paths") == 0) {

Note that this change effectively isn't only faulting, but it inverts the logic of the test: as it is now true when the string "Paths" is not found in name (i.e. when strstr returns NULL).

Under that assumption, rfind(.., 0) is the most efficient way to test for the string to start with "Paths", as find() would continue to search the rest of name, just to discard the result afterwards.

@cluck
Copy link
Contributor Author

cluck commented Oct 26, 2024

I wonder what was the motivation for the change in the first place?

cluck pushed a commit to cluck/bomutils that referenced this issue Oct 26, 2024
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

No branches or pull requests

2 participants