Skip to content

[CWE-476-690] cborpretty: fix indicator uncheck on NULL before dereference #304

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.

Sign up for GitHub

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

[CWE-476-690] cborpretty: fix indicator uncheck on NULL before dereference #304

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 12 additions & 3 deletions src/cborpretty.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -345,6 +345,8 @@ static CborError value_to_pretty(CborStreamFunction stream, void *out, CborValue
/* recursive type */
CborValue recursed;
const char *indicator = get_indicator(it, flags);
if (!indicator)
return err;
Comment on lines +348 to +349
Copy link
Member

@thiagomacieira thiagomacieira Aug 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This isn't correct: at this point, err = CborNoError but there was an error.

But I don't think the error condition is possible in the first place.

Copy link
Author

@GermanAizek GermanAizek Aug 21, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But I don't think the error condition is possible in the first place.

if you deliberately try to do this, it is possible, but even if you do it, it will lead to the usual segfault program. Rights?

Copy link
Member

@thiagomacieira thiagomacieira Aug 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No. I am saying I don't think it's possible to segfault the program even if you deliberately try.

Can you show an example that would trigger this condition?

const char *space = *indicator ? " " : indicator;

err = stream(out, "%c%s%s", type == CborArrayType ? '[' : '{', indicator, space);
Expand Down Expand Up @@ -389,8 +391,12 @@ static CborError value_to_pretty(CborStreamFunction stream, void *out, CborValue
err = stream(out, "-18446744073709551616");
}
}
if (!err)
err = stream(out, "%s", get_indicator(it, flags));
if (!err) {
const char *indicator = get_indicator(it, flags);
if (!indicator)
return err;
err = stream(out, "%s", indicator);
}
break;
}

Expand Down Expand Up @@ -452,7 +458,10 @@ static CborError value_to_pretty(CborStreamFunction stream, void *out, CborValue
case CborTagType: {
CborTag tag;
cbor_value_get_tag(it, &tag); /* can't fail */
err = stream(out, "%" PRIu64 "%s(", tag, get_indicator(it, flags));
const char *indicator = get_indicator(it, flags);
if (!indicator)
return err;
err = stream(out, "%" PRIu64 "%s(", tag, indicator);
if (!err)
err = cbor_value_advance_fixed(it);
if (!err && recursionsLeft > 0)
Expand Down