Skip to content

[FIXED] microService_AddEndpoint() could crash if subject is invalid - #831

Merged
levb merged 1 commit into
mainfrom
fix_use_after_free
Jan 15, 2025
Merged

[FIXED] microService_AddEndpoint() could crash if subject is invalid#831
levb merged 1 commit into
mainfrom
fix_use_after_free

Conversation

@kozlovic

Copy link
Copy Markdown
Member

Saw that looking at the build log in GitHub actions:

/home/runner/work/nats.c/nats.c/src/micro.c: In function ‘micro_add_endpoint’:
/home/runner/work/nats.c/nats.c/src/micro.c:153:16: warning: pointer used after ‘free’ [-Wuse-after-free]
  153 |         return microError_Wrapf(micro_ErrorInvalidArg, "invalid subject '%s' for endpoint '%s'", fullSubject, cfg->Name);
      |                ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
In file included from /home/runner/work/nats.c/nats.c/src/microp.h:18,
                 from /home/runner/work/nats.c/nats.c/src/micro.c:16:
/home/runner/work/nats.c/nats.c/src/mem.h:28:29: note: call to ‘free’ here
   28 | #define NATS_FREE(p)        free((p))
      |                             ^~~~~~~~~
/home/runner/work/nats.c/nats.c/src/micro.c:152:9: note: in expansion of macro ‘NATS_FREE’
  152 |         NATS_FREE(fullSubject);
      |         ^~~~~~~~~

Added an invalid subject endpoint config in one of the test to verify the issue and the fix.

Signed-off-by: Ivan Kozlovic ivan@synadia.com

Saw that looking at the build log in GitHub actions:
```
/home/runner/work/nats.c/nats.c/src/micro.c: In function ‘micro_add_endpoint’:
/home/runner/work/nats.c/nats.c/src/micro.c:153:16: warning: pointer used after ‘free’ [-Wuse-after-free]
  153 |         return microError_Wrapf(micro_ErrorInvalidArg, "invalid subject '%s' for endpoint '%s'", fullSubject, cfg->Name);
      |                ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
In file included from /home/runner/work/nats.c/nats.c/src/microp.h:18,
                 from /home/runner/work/nats.c/nats.c/src/micro.c:16:
/home/runner/work/nats.c/nats.c/src/mem.h:28:29: note: call to ‘free’ here
   28 | #define NATS_FREE(p)        free((p))
      |                             ^~~~~~~~~
/home/runner/work/nats.c/nats.c/src/micro.c:152:9: note: in expansion of macro ‘NATS_FREE’
  152 |         NATS_FREE(fullSubject);
      |         ^~~~~~~~~
```

Added an invalid subject endpoint config in one of the test to verify
the issue and the fix.

Signed-off-by: Ivan Kozlovic <ivan@synadia.com>
@kozlovic
kozlovic requested a review from levb January 13, 2025 18:03
@codecov

codecov Bot commented Jan 13, 2025

Copy link
Copy Markdown

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 70.49%. Comparing base (1553d4a) to head (d1df7ee).
Report is 34 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #831      +/-   ##
==========================================
+ Coverage   68.71%   70.49%   +1.78%     
==========================================
  Files          39       47       +8     
  Lines       15207    15368     +161     
  Branches     3143     3149       +6     
==========================================
+ Hits        10449    10834     +385     
+ Misses       1700     1499     -201     
+ Partials     3058     3035      -23     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@levb levb left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

LGTM. thx @kozlovic !

@levb
levb merged commit 022a510 into main Jan 15, 2025
@levb
levb deleted the fix_use_after_free branch January 15, 2025 14:32
github-actions Bot pushed a commit that referenced this pull request Jan 15, 2025
levb pushed a commit that referenced this pull request Feb 28, 2025
…831)

Saw that looking at the build log in GitHub actions:
```
/home/runner/work/nats.c/nats.c/src/micro.c: In function ‘micro_add_endpoint’:
/home/runner/work/nats.c/nats.c/src/micro.c:153:16: warning: pointer used after ‘free’ [-Wuse-after-free]
  153 |         return microError_Wrapf(micro_ErrorInvalidArg, "invalid subject '%s' for endpoint '%s'", fullSubject, cfg->Name);
      |                ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
In file included from /home/runner/work/nats.c/nats.c/src/microp.h:18,
                 from /home/runner/work/nats.c/nats.c/src/micro.c:16:
/home/runner/work/nats.c/nats.c/src/mem.h:28:29: note: call to ‘free’ here
   28 | #define NATS_FREE(p)        free((p))
      |                             ^~~~~~~~~
/home/runner/work/nats.c/nats.c/src/micro.c:152:9: note: in expansion of macro ‘NATS_FREE’
  152 |         NATS_FREE(fullSubject);
      |         ^~~~~~~~~
```

Added an invalid subject endpoint config in one of the test to verify
the issue and the fix.

Signed-off-by: Ivan Kozlovic <ivan@synadia.com>
@levb levb removed the cherry-pick label Feb 28, 2025
@levb levb mentioned this pull request Feb 28, 2025
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