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

nng_recv() appears to be vulnerable to buffer overran attack #1603

Closed
kagalenko-m-b opened this issue Jun 25, 2022 · 4 comments
Closed

nng_recv() appears to be vulnerable to buffer overran attack #1603

kagalenko-m-b opened this issue Jun 25, 2022 · 4 comments

Comments

@kagalenko-m-b
Copy link

kagalenko-m-b commented Jun 25, 2022

nng_recv(), when called with NNG_FLAG_ALLOC, allocates buffer for the received data. However, there does not appear to be an way to limit the maximum size of this
buffer. This behaviour is similar to the strcpy() operator, which is a notorious source of security vulnerabilities.

Therefore, net_recv() must be deprecated and replaced with the call
int nng_recvn(nng_socket s, void *data, size_t *sizep, size_t max_size, int flags);

@gdamore
Copy link
Contributor

gdamore commented Jun 25, 2022

Please see NNG_OPT_RECVMAXSZ which exists specifically to address this concern.

@kagalenko-m-b
Copy link
Author

kagalenko-m-b commented Jun 29, 2022

Thanks, NNG_OPT_RECVMAXSZ does address the issue to a large extent. Two concerns still remain:

  • It's not mentioned in documentation on net_recv()
  • It's different from familiar POSIX interfaces like strncpy() or snprintf()

@gdamore
Copy link
Contributor

gdamore commented Jun 29, 2022

A few things:

  1. it's nng_recv not net_recv. (Same issue comes up with nng_recvmsg btw.)
  2. semantically, it's not possible to enforce the policy at nng_recv, because by the time nng_recv is called to process the message, it has already been allocated and injected into an internal queue. We need to do it earlier.
  3. the semantics are far different from POSIX, and expecting a POSIX familiarity here is setting yourself up for heartache. Trying to align is only going to further add to confusion and disappointment. (See the POSIX APIs section in https://github.com/nanomsg/nng/blob/master/docs/RATIONALE.adoc for some more of my thoughts about this.)

@gdamore gdamore closed this as completed Jun 29, 2022
@gdamore
Copy link
Contributor

gdamore commented Jun 29, 2022

(I would be willing to accept a PR for enhancing the docs for nng_recv though.)

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