Skip to content

RDKE: 61924 / RDKB: 66749 Mythos Defect Remediation - RRD - #262

Open
Kaavyashree2 wants to merge 9 commits into
developfrom
feature/rrd_mythosfix
Open

Kaavyashree2 wants to merge 9 commits into
developfrom
feature/rrd_mythosfix

Conversation

@Kaavyashree2

@Kaavyashree2 Kaavyashree2 commented Nov 4, 2026 •

Copy link
Copy Markdown
  • Sanitized IssueType inputs in shell commands
  • Improved validation of MsgPack object types returned by finder()
  • Integer overflow prevention

@Kaavyashree2
Kaavyashree2 requested a review from a team as a code owner November 4, 2026 09:45
Copilot AI balanced review requested due to automatic review settings November 4, 2026 09:45
@github-actions

github-actions Bot commented Nov 4, 2026

Copy link
Copy Markdown
Contributor

Code Coverage Summary

                               Total:|84.0%   6280|97.3%  1668|    -      0

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟢 Approval recommended

There are no active review issues for the change that sanitizes issue types used in log archive filenames.

No open issues 🎉

What changed in this PR

This PR hardens issue-type handling in the remote-debugger log upload script before the value is included in archive filenames.

Changes:

  • Replaces unquoted echo with quoted printf to avoid word splitting and wildcard expansion.
  • Converts the issue type to uppercase and restricts it to A-Z, digits, underscores, and hyphens.
File Description
scripts/​uploadRRDLogs.sh Sanitizes the issue type used in log archive filenames.

Review effort: Balanced

Copilot AI balanced review requested due to automatic review settings November 4, 2026 09:48

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Changes recommended

The new null assertion always fails because the unchanged decoder returns the matching string value despite the requested integer type, without setting errno.

1 open issue

Review effort: Balanced

Comment thread src/unittest/rrdUnitTestRunner.cpp Resolved
Copilot AI balanced review requested due to automatic review settings November 4, 2026 09:54

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Changes recommended

The decoder contains invalid syntax that blocks compilation, an unchecked metadata pointer that can cause a null dereference, and an unhandled overflow result that can lead to a heap buffer overflow.

4 open issues

Review effort: Balanced

Comment thread src/rrdMsgPackDecoder.c Resolved
Comment thread src/rrdMsgPackDecoder.c Resolved
Comment thread src/rrdMsgPackDecoder.c Resolved
Copilot AI balanced review requested due to automatic review settings November 4, 2026 10:15

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🔵 Needs a closer look

The decoder contains a syntax error that prevents compilation and unchecked metadata pointers that can cause malformed input to crash the daemon.

4 open issues

Review effort: Balanced

Copilot AI balanced review requested due to automatic review settings November 4, 2026 10:21
@github-actions

github-actions Bot commented Nov 4, 2026

Copy link
Copy Markdown
Contributor

Code Coverage Summary

                               Total:|83.9%   6285|97.3%  1668|    -      0

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Changes recommended

Unresolved critical issues allow malformed metadata to crash the daemon and unchecked command lengths to overflow downstream storage or allocations, risking out-of-bounds writes.

5 open issues
1 resolved since last review

Review effort: Balanced

Comment thread src/rrdMsgPackDecoder.c Resolved
Comment thread src/rrdMsgPackDecoder.c Resolved
@Kaavyashree2 Kaavyashree2 changed the title Mythos fix RDKE: 61924 / RDKB: 66749 Mythos Defect Remediation - RRD Nov 4, 2026
Copilot AI balanced review requested due to automatic review settings November 6, 2026 09:09

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Changes recommended

Malformed metadata can cause null-pointer dereferences that crash the daemon, and the new overflow test fails on 64-bit hosts because its string length is truncated.

1 open issue
5 resolved since last review

Review effort: Balanced

Comment thread src/unittest/rrdUnitTestRunner.cpp Resolved
Copilot AI balanced review requested due to automatic review settings November 6, 2026 09:27

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🔵 Needs a closer look

Malformed metadata can trigger a NULL-pointer dereference that crashes the daemon, and the new overflow test fails on 64-bit hosts due to truncation of the uint32_t string length.

1 open issue

Review effort: Balanced

Copilot AI balanced review requested due to automatic review settings November 6, 2026 09:30

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🔵 Needs a closer look

Malformed metadata can trigger a NULL dereference that crashes the daemon, and the new overflow test incorrectly expects zero on 64-bit platforms.

1 open issue

Review effort: Balanced

Copilot AI balanced review requested due to automatic review settings November 6, 2026 09:40
@github-actions

github-actions Bot commented Nov 6, 2026

Copy link
Copy Markdown
Contributor

Code Coverage Summary

                               Total:|83.9%   6293|97.3%  1668|    -      0

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Changes recommended

Malformed metadata can trigger NULL dereferences that crash the daemon, and the length checks still allow allocation-size overflow on 32-bit targets.

1 open issue
1 resolved since last review

Review effort: Balanced

Comment on lines +3407 to +3432
/*

TEST(GetReqTotalSizeOfIssueTypesStrTest, DetectsSizeOverflow)
{
msgpack_object obj_array[2];

obj_array[0].type = MSGPACK_OBJECT_STR;
obj_array[0].via.str.size = SIZE_MAX - 1;

obj_array[1].type = MSGPACK_OBJECT_STR;
obj_array[1].via.str.size = 10;

printf("SIZE_MAX=%zu\n", (size_t)SIZE_MAX);
printf("obj0 size=%zu\n", (size_t)obj_array[0].via.str.size);
printf("obj1 size=%zu\n", (size_t)obj_array[1].via.str.size);

msgpack_object_array arr = {2, obj_array};

size_t result = getReqTotalSizeOfIssueTypesStr(&arr);

printf("result=%zu\n", result);


EXPECT_EQ(result, 0u);
}
*/
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.

2 participants