镜像站点 · 本页由第三方 GitHub 只读镜像提供,非 GitHub 官方站点,不接受任何登录或凭据输入。前往 github.com
Skip to content

Let's make a plan for bad getters and toString() methods #12372

Description

@cjihrig

Version: master
Platform: all
Subsystem: src

It seems like these types of issues get opened pretty frequently. Node accepts an object as input, and one of the properties on that object is a getter that throws an error. Similar problems exist with toString() and others. It's usually reported as a security issue that bypasses JavaScript validation (when we have some in place) and crashes in the binding layer.

I tried to fix one such issue and was told that instead of fixing ad hoc, we should come up with a plan. I agree with that idea. So, let's come up with a plan. Should we ignore the problem? Ensure that getters and other potential problematic methods are corrected in the JS layer? Something else?

Activity

  1. Trott commented on Apr 12, 2017

    @Trott
    Member
  2. addaleax commented on Apr 12, 2017

    @addaleax
    Member

    I’m good with the approach that e.g. #12371 is taking – handle everything properly in the C++ layer, forwarding possible exceptions to JS if they are encountered.

  3. added
    c++Issues and PRs that require attention from people who are familiar with C++.
    on Apr 12, 2017
  4. added
    securityIssues and PRs related to security.
    on Apr 12, 2017
  5. aqrln commented on Apr 12, 2017

    @aqrln
    Contributor

    Applied the "security" label as it may be concerned by this issue in, at least, some cases; please remove it if it's not relevant.

  6. addaleax commented on Apr 12, 2017

    @addaleax
    Member

    I think so far all collaborators have indicated in the discussions here that those issues can’t reasonably be considered security issues.

  7. removed
    securityIssues and PRs related to security.
    on Apr 12, 2017
  8. DemiMarie commented on Apr 12, 2017

    @DemiMarie

    One (rather extreme) option is to:

    • quit using -fno-exceptions for Node's C++ code
    • throw C++ exceptions when JS exceptions occur
    • catch and ignore the exception at the V8 callback interface.
  9. bnoordhuis commented on Apr 12, 2017

    @bnoordhuis
    Member

    I personally consider it a non-issue. I'd be more amenable if it was something you hit by accident but so far it's all been people playing at security researcher.

  10. tniessen commented on Apr 12, 2017

    @tniessen
    Member

    I agree with @addaleax: I don't see a problem fixing it ad hoc, this does not appear to be a design problem. None of the reported issues I came across pose any real-world danger.

  11. bnoordhuis commented on Apr 13, 2017

    @bnoordhuis
    Member

    @tniessen By the way, your first comment (the one you deleted) about gradually updating everything to MaybeLocal<Value> is on the ball. It's happening slowly but surely whenever we make updates.

  12. cjihrig commented on Apr 14, 2017

    @cjihrig
    ContributorAuthor

    Thanks for the feedback. I'll close the issue. Feel free to continue the discussion.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    c++Issues and PRs that require attention from people who are familiar with C++.

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions