Skip to content

fix: add NULL pointer check in rcl_client_bindings.cpp (CWE-476) - #1613

Merged
minggangw merged 2 commits into
RobotWebTools:developfrom
anupamme:fix-repo-rclnodejs-cwe-476-client-malloc-null-check
Sep 21, 2026
Merged

minggangw merged 2 commits into
RobotWebTools:developfrom
anupamme:fix-repo-rclnodejs-cwe-476-client-malloc-null-check

Conversation

@anupamme

@anupamme anupamme commented Sep 20, 2026

Copy link
Copy Markdown
Contributor

The CreateClient function allocates memory for rcl_client_t using malloc() without checking if the allocation succeeded. If malloc returns NULL due to memory exhaustion, the code immediately dereferences the NULL pointer, causing a segmentation fault and application crash. The affected code is src/rcl_client_bindings.cpp:43. This change is the fix I would apply.

Reference: CWE-476

Fixes #1616

What changed

  • src/rcl_client_bindings.cpp

Verification

No automated check could be run against this repository, so this change is unverified beyond review. Please treat it as a suggestion.


Automated security fix by OrbisAI Security

Automated security fix generated by OrbisAI Security
@minggangw

Copy link
Copy Markdown
Member

Thanks for submitting the PR, please address the lint error below, and create an issue to be linked with the PR.

  /__w/rclnodejs/rclnodejs/src/rcl_client_bindings.cpp:44:  Lines should be <= 80 characters long  [whitespace/line_length] [2]
  
  /__w/rclnodejs/rclnodejs/scripts/cpplint.cjs:38
          throw Error('cpplint failed.');
          ^
  
  Error: cpplint failed.
      at /__w/rclnodejs/rclnodejs/scripts/cpplint.cjs:38:15
      at ChildProcess.exithandler (node:child_process:424:5)
      at ChildProcess.emit (node:events:514:28)
      at maybeClose (node:internal/child_process:1141:16)
      at ChildProcess._handle.onexit (node:internal/child_process:306:5)

Line 44 exceeded the 80-character limit, causing cpplint to fail in CI.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@anupamme

Copy link
Copy Markdown
Contributor Author

Thanks for submitting the PR, please address the lint error below, and create an issue to be linked with the PR.

  /__w/rclnodejs/rclnodejs/src/rcl_client_bindings.cpp:44:  Lines should be <= 80 characters long  [whitespace/line_length] [2]
  
  /__w/rclnodejs/rclnodejs/scripts/cpplint.cjs:38
          throw Error('cpplint failed.');
          ^
  
  Error: cpplint failed.
      at /__w/rclnodejs/rclnodejs/scripts/cpplint.cjs:38:15
      at ChildProcess.exithandler (node:child_process:424:5)
      at ChildProcess.emit (node:events:514:28)
      at maybeClose (node:internal/child_process:1141:16)
      at ChildProcess._handle.onexit (node:internal/child_process:306:5)

Addressed. Pls review.

@coveralls

Copy link
Copy Markdown

Coverage Status

coverage: 91.136%. remained the same — anupamme:fix-repo-rclnodejs-cwe-476-client-malloc-null-check into RobotWebTools:develop

@minggangw minggangw left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM!

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot review overview

🟢 Approval recommended

The guard safely handles allocation failure before dereferencing the pointer.

Review effort: Balanced
Findings: None

What changed in this PR

Prevents a native crash when client memory allocation fails.

Changes:

  • Adds a null check before initializing the allocated rcl_client_t.
File Description
src/​rcl_client_bindings.cpp Throws a JavaScript-visible error when client allocation fails.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@minggangw
minggangw merged commit 7fec48d into RobotWebTools:develop Sep 21, 2026
17 checks passed
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.

CreateClient: missing NULL check after malloc() for rcl_client_t (CWE-476)

4 participants