Skip to content

[CVE-2016-5180] ares_create_query: avoid single-byte buffer overwrite #8849

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 0 commits into from
Closed

[CVE-2016-5180] ares_create_query: avoid single-byte buffer overwrite #8849

wants to merge 0 commits into from

Conversation

sgallagher
Copy link
Contributor

Checklist
  • make -j8 test (UNIX), or vcbuild test nosign (Windows) passes
  • commit message follows commit guidelines
Affected core subsystem(s)

bundled c-ares

Description of change

Avoid single-byte buffer overwrite when the name ends with an escaped dot.

CVE-2016-5180

Bug: https://c-ares.haxx.se/adv_20160929.html

@nodejs-github-bot nodejs-github-bot added the cares Issues and PRs related to the c-ares dependency or the cares_wrap binding. label Sep 29, 2016
@MylesBorins
Copy link
Contributor

MylesBorins commented Sep 29, 2016 •
edited
Loading

I can confirm the e8dd387 is identical to the patch found at https://c-ares.haxx.se/CVE-2016-5180.patch

LGTM

@MylesBorins MylesBorins added the security Issues and PRs related to security. label Sep 29, 2016
jasnell
jasnell approved these changes Sep 29, 2016
View reviewed changes
Copy link
Member

@jasnell jasnell left a comment

Choose a reason for hiding this comment

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

LGTM

jbergstroem
jbergstroem approved these changes Sep 29, 2016
View reviewed changes
Copy link
Member

@jbergstroem jbergstroem left a comment

Choose a reason for hiding this comment

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

LGTM

@jbergstroem
Copy link
Member

CI: https://ci.nodejs.org/job/node-test-commit/5365/

indutny
indutny approved these changes Sep 29, 2016
View reviewed changes
Copy link
Member

@indutny indutny left a comment

Choose a reason for hiding this comment

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

LGTM

@MylesBorins
Copy link
Contributor

MylesBorins commented Sep 29, 2016 •
edited
Loading

windows failure is infra related https://ci.nodejs.org/job/node-test-binary-windows/4102/RUN_SUBSET=3,VS_VERSION=vcbt2015,label=win10/tapTestReport/test.tap-238/

/cc @nodejs/platform-windows re: flaky test

@Trott
Copy link
Member

Trott commented Sep 30, 2016

Aside: I think @geek has a PR in to fix that test. #8848

@jbergstroem
Copy link
Member

I'll land this in 24h unless anyone has any objections.

@MylesBorins
Copy link
Contributor

once it lands I'll deal with backporting

On Fri, Sep 30, 2016, 1:52 AM Johan Bergström [email protected]
wrote:

I'll land this in 24h unless anyone has any objections.

—
You are receiving this because you commented.

Reply to this email directly, view it on GitHub
#8849 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/AAecV9Jgf-c-FaTgZYMHzDycGyTb9ZC8ks5qvKOSgaJpZM4KKT9Z
.

@MylesBorins
Copy link
Contributor

Another run of CI: https://ci.nodejs.org/job/node-test-pull-request/4332/

@jbergstroem
Copy link
Member

Test failures look unrelated.

@jbergstroem jbergstroem closed this Oct 2, 2016
@jbergstroem
Copy link
Member

wtf -- PR got closed when I pushed to the remote branch. Merged in 68c4c71.

jbergstroem pushed a commit that referenced this pull request Oct 2, 2016
@bagder @jbergstroem
ares_create_query: avoid single-byte buffer overwrite
68c4c71 
Incorrect string length calculation when passing escaped dot.

- CVE: CVE-2016-5180
- Upstream bug: https://c-ares.haxx.se/adv_20160929.html

PR-URL: #8849
Reviewed-By: Myles Borins <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Johan Bergström <[email protected]>
Reviewed-By: Fedor Indutny <[email protected]>
@mscdex
Copy link
Contributor

mscdex commented Oct 3, 2016

This should have had a better subsystem name, like cares: instead of ares_create_query:.

@jbergstroem
Copy link
Member

@mscdex true, apologies.

jasnell pushed a commit that referenced this pull request Oct 6, 2016
@bagder @jasnell
ares_create_query: avoid single-byte buffer overwrite
c5643a8 
Incorrect string length calculation when passing escaped dot.

- CVE: CVE-2016-5180
- Upstream bug: https://c-ares.haxx.se/adv_20160929.html

PR-URL: #8849
Reviewed-By: Myles Borins <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Johan Bergström <[email protected]>
Reviewed-By: Fedor Indutny <[email protected]>
@MylesBorins MylesBorins self-assigned this Oct 6, 2016
@MylesBorins
Copy link
Contributor

So it looks like v4.x is using care v1.10.1 and as such is affected, this patch is not landing cleanly though. I'll manually backport

Fishrock123 pushed a commit that referenced this pull request Oct 11, 2016
@bagder @Fishrock123
ares_create_query: avoid single-byte buffer overwrite
23a851d 
Incorrect string length calculation when passing escaped dot.

- CVE: CVE-2016-5180
- Upstream bug: https://c-ares.haxx.se/adv_20160929.html

PR-URL: #8849
Reviewed-By: Myles Borins <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Johan Bergström <[email protected]>
Reviewed-By: Fedor Indutny <[email protected]>
MylesBorins pushed a commit to MylesBorins/node that referenced this pull request Oct 11, 2016
@bagder
deps: avoid single-byte buffer overwrite
ffcd24c 
Incorrect string length calculation when passing escaped dot.

- CVE: CVE-2016-5180
- Upstream bug: https://c-ares.haxx.se/adv_20160929.html

Ref: nodejs#9037
PR-URL: nodejs#8849
Reviewed-By: Myles Borins <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Johan Bergström <[email protected]>
Reviewed-By: Fedor Indutny <[email protected]>
MylesBorins pushed a commit that referenced this pull request Oct 14, 2016
@bagder @MylesBorins
deps: avoid single-byte buffer overwrite
1f900b6 
Incorrect string length calculation when passing escaped dot.

- CVE: CVE-2016-5180
- Upstream bug: https://c-ares.haxx.se/adv_20160929.html

Ref: #9037
PR-URL: #8849
Reviewed-By: Myles Borins <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Johan Bergström <[email protected]>
Reviewed-By: Fedor Indutny <[email protected]>
rvagg pushed a commit that referenced this pull request Oct 15, 2016
@bagder @rvagg
deps: avoid single-byte buffer overwrite
c5b095e 
Incorrect string length calculation when passing escaped dot.

- CVE: CVE-2016-5180
- Upstream bug: https://c-ares.haxx.se/adv_20160929.html

Ref: #9037
PR-URL: #8849
Reviewed-By: Myles Borins <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Johan Bergström <[email protected]>
Reviewed-By: Fedor Indutny <[email protected]>
rvagg added a commit to rvagg/io.js that referenced this pull request Oct 15, 2016
@rvagg
deps: c-ares, avoid single-byte buffer overwrite
c1dc01c 
Backport of nodejs#8849 for c-ares
1.9.0.

Incorrect string length calculation when passing escaped dot.

- CVE: CVE-2016-5180
- Upstream bug: https://c-ares.haxx.se/adv_20160929.html
This was referenced Oct 15, 2016
Closed
Merged
rvagg pushed a commit to rvagg/io.js that referenced this pull request Oct 18, 2016
@bagder @rvagg
deps: avoid single-byte buffer overwrite
f3c63e7 
Incorrect string length calculation when passing escaped dot.

- CVE: CVE-2016-5180
- Upstream bug: https://c-ares.haxx.se/adv_20160929.html

Ref: nodejs#9037
PR-URL: nodejs#8849
Reviewed-By: Myles Borins <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Johan Bergström <[email protected]>
Reviewed-By: Fedor Indutny <[email protected]>
@rvagg rvagg mentioned this pull request Oct 18, 2016
Closed
rvagg added a commit to rvagg/io.js that referenced this pull request Oct 18, 2016
@rvagg
deps: c-ares, avoid single-byte buffer overwrite
a14a6a3 
Backport of nodejs#8849 for c-ares
1.9.0.

Incorrect string length calculation when passing escaped dot.

- CVE: CVE-2016-5180
- Upstream bug: https://c-ares.haxx.se/adv_20160929.html

PR-URL: nodejs#9108
Reviewed-By: Ben Noordhuis <[email protected]>
@addaleax addaleax mentioned this pull request Mar 7, 2017
Closed
@yosifkit yosifkit mentioned this pull request Mar 13, 2017
Closed
@chorrell chorrell mentioned this pull request Apr 4, 2017
Closed
@bnoordhuis bnoordhuis mentioned this pull request Apr 20, 2017
Closed
@snyk-bot snyk-bot mentioned this pull request Apr 24, 2023
Open
@aliscco aliscco mentioned this pull request Apr 28, 2023
Open
@aliscco aliscco mentioned this pull request Jun 23, 2023
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jasnell jasnell jasnell approved these changes

@cjihrig cjihrig cjihrig approved these changes

+3 more reviewers

@jbergstroem jbergstroem jbergstroem approved these changes

@indutny indutny indutny approved these changes

@MylesBorins MylesBorins MylesBorins approved these changes

Reviewers whose approvals may not affect merge requirements
Assignees

@MylesBorins MylesBorins

Labels
cares Issues and PRs related to the c-ares dependency or the cares_wrap binding. security Issues and PRs related to security.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
9 participants
@sgallagher @MylesBorins @jbergstroem @Trott @mscdex @indutny @jasnell @cjihrig @nodejs-github-bot