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

Use length to ensure null chars do not cause early termination of C string copies/reads #272

Merged
merged 13 commits into from
Jan 12, 2022

Conversation

genevieve
Copy link
Collaborator

@genevieve genevieve commented Jan 7, 2022

Related to #192

@codecov
Copy link

codecov bot commented Jan 7, 2022

Codecov Report

Merging #272 (1361f8e) into master (cb8779b) will decrease coverage by 0.00%.
The diff coverage is 83.33%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #272      +/-   ##
==========================================
- Coverage   95.88%   95.87%   -0.01%     
==========================================
  Files          17       17              
  Lines         583      582       -1     
==========================================
- Hits          559      558       -1     
  Misses         15       15              
  Partials        9        9              
Impacted Files Coverage Δ
value.go 94.82% <83.33%> (-0.03%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update cb8779b...1361f8e. Read the comment docs.

@genevieve genevieve closed this Jan 10, 2022
@dylanahsmith
Copy link
Collaborator

strnlen treats the input as a null terminated string, so counts the number of characters before the null byte to get the length. We need to preserve the length rather than trying to get it back from a char*

@genevieve genevieve reopened this Jan 11, 2022
value_test.go Outdated Show resolved Hide resolved
@genevieve genevieve marked this pull request as ready for review January 11, 2022 17:04
Copy link
Collaborator

@dylanahsmith dylanahsmith left a comment

Choose a reason for hiding this comment

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

This PR shouldn't mark issue #192 as fixed, because it just addresses a couple of important instances of the issue. That ok for keeping the PR focused, but the issue will still remain with usage of CopyString and GoString that can easily be cargo culted to do the wrong thing.

v8go.cc Show resolved Hide resolved
value.go Outdated Show resolved Hide resolved
value_test.go Outdated Show resolved Hide resolved
value_test.go Outdated Show resolved Hide resolved
Genevieve and others added 2 commits January 11, 2022 13:24
@genevieve genevieve merged commit 1d433f7 into rogchap:master Jan 12, 2022
@genevieve genevieve deleted the sws-722 branch January 12, 2022 18:41
ajvpot referenced this pull request in ajvpot/lockfileparsergo Jan 20, 2023
[![Mend
Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com)

This PR contains the following updates:

| Package | Type | Update | Change |
|---|---|---|---|
| [rogchap.com/v8go](https://togithub.com/rogchap/v8go) | require |
minor | `v0.7.0` -> `v0.8.0` |

---

### Release Notes

<details>
<summary>rogchap/v8go</summary>

### [`v0.8.0`](https://togithub.com/rogchap/v8go/releases/tag/v0.8.0)

[Compare
Source](https://togithub.com/rogchap/v8go/compare/v0.7.0...v0.8.0)

Full changelog ⇒
[v0.8.0](https://togithub.com/rogchap/v8go/blob/master/CHANGELOG.md#v080---2023-01-19)

#### What's Changed

- Allocate profile node children array using count by
[@&#8203;genevieve](https://togithub.com/genevieve) in
[https://github.com/rogchap/v8go/pull/256](https://togithub.com/rogchap/v8go/pull/256)
- Build v8 for linux_arm64 by [@&#8203;epk](https://togithub.com/epk) in
[https://github.com/rogchap/v8go/pull/223](https://togithub.com/rogchap/v8go/pull/223)
- CHANGELOG: Highlight the subtlety of the Function Call breaking change
by [@&#8203;dylanahsmith](https://togithub.com/dylanahsmith) in
[https://github.com/rogchap/v8go/pull/259](https://togithub.com/rogchap/v8go/pull/259)
- change -fpic to -fPIC in cgo.go by
[@&#8203;iwind](https://togithub.com/iwind) in
[https://github.com/rogchap/v8go/pull/263](https://togithub.com/rogchap/v8go/pull/263)
- Remove unnecessary nested Locker and Isolate::Scope use by
[@&#8203;dylanahsmith](https://togithub.com/dylanahsmith) in
[https://github.com/rogchap/v8go/pull/275](https://togithub.com/rogchap/v8go/pull/275)
- Upgrade V8 binaries for 9.7.106.18 version by
[@&#8203;github-actions](https://togithub.com/github-actions) in
[https://github.com/rogchap/v8go/pull/264](https://togithub.com/rogchap/v8go/pull/264)
- Use length to ensure null chars do not cause early termination of C
string copies/reads by
[@&#8203;genevieve](https://togithub.com/genevieve) in
[https://github.com/rogchap/v8go/pull/272](https://togithub.com/rogchap/v8go/pull/272)
- Fix Object.Set with an empty key string by
[@&#8203;dylanahsmith](https://togithub.com/dylanahsmith) in
[https://github.com/rogchap/v8go/pull/276](https://togithub.com/rogchap/v8go/pull/276)
- Upgrade V8 binaries for 9.7.106.19 version by
[@&#8203;github-actions](https://togithub.com/github-actions) in
[https://github.com/rogchap/v8go/pull/278](https://togithub.com/rogchap/v8go/pull/278)
- Fix typo in promise.go by
[@&#8203;lukasmalkmus](https://togithub.com/lukasmalkmus) in
[https://github.com/rogchap/v8go/pull/310](https://togithub.com/rogchap/v8go/pull/310)
- Updating context documentation by
[@&#8203;ryanmurakami](https://togithub.com/ryanmurakami) in
[https://github.com/rogchap/v8go/pull/335](https://togithub.com/rogchap/v8go/pull/335)
- Add additional CPUProfile values by
[@&#8203;ryanmurakami](https://togithub.com/ryanmurakami) in
[https://github.com/rogchap/v8go/pull/341](https://togithub.com/rogchap/v8go/pull/341)
- v8.Value becomes manually releaseable by
[@&#8203;fizx](https://togithub.com/fizx) in
[https://github.com/rogchap/v8go/pull/361](https://togithub.com/rogchap/v8go/pull/361)

#### New Contributors

- [@&#8203;iwind](https://togithub.com/iwind) made their first
contribution in
[https://github.com/rogchap/v8go/pull/263](https://togithub.com/rogchap/v8go/pull/263)
- [@&#8203;lukasmalkmus](https://togithub.com/lukasmalkmus) made their
first contribution in
[https://github.com/rogchap/v8go/pull/310](https://togithub.com/rogchap/v8go/pull/310)
- [@&#8203;ryanmurakami](https://togithub.com/ryanmurakami) made their
first contribution in
[https://github.com/rogchap/v8go/pull/335](https://togithub.com/rogchap/v8go/pull/335)
- [@&#8203;fizx](https://togithub.com/fizx) made their first
contribution in
[https://github.com/rogchap/v8go/pull/361](https://togithub.com/rogchap/v8go/pull/361)

**Full Changelog**:
rogchap/v8go@v0.7.0...v0.8.0

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined),
Automerge - At any time (no schedule defined).

🚦 **Automerge**: Disabled by config. Please merge this manually once you
are satisfied.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the
rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update
again.

---

- [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check
this box

---

This PR has been generated by [Mend
Renovate](https://www.mend.io/free-developer-tools/renovate/). View
repository job log
[here](https://app.renovatebot.com/dashboard#github/ajvpot/lockfileparsergo).

<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNC4xMDUuNCIsInVwZGF0ZWRJblZlciI6IjM0LjEwNS40In0=-->

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
wowngasb added a commit to wowngasb/v8go that referenced this pull request Jan 19, 2024
commit cee5f84
Author: Jonathan Geddes <[email protected]>
Date:   Mon Apr 10 10:09:13 2023 -0600

    shared array buffers (rogchap#378)

    * Initial support for SharedArrayBuffer

commit 0e40e6e
Author: Randy Goodman <[email protected]>
Date:   Mon Mar 20 19:27:55 2023 -0400

    v0.9.0 (rogchap#376)

commit be4ff5e
Author: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Date:   Tue Mar 21 07:17:39 2023 +1100

    Upgrade V8 binaries for 11.1.277.13 version (rogchap#373)

commit 02e1763
Author: Jacques Nadeau <[email protected]>
Date:   Mon Jan 30 06:51:45 2023 -0800

    Upgrade V8 binaries for 10.9.194.9 version **working** (rogchap#363)

    * Upgrade V8 binaries for 10.9.194.9 version

    * Fixes to support newest stable v8.

    - Update github workflow to use go 18 & 19
    - Update github workflow to use macos-latest
    - Update github build workflow to use ubuntu 22.04
    - Add gitignore for jetbrains and .gclient_previous files
    - Switch cgo build to C++17 and enable sandbox at build time
    - Update test with update to date error message
    - Remove no longer supported build flag.
    - Move initialization to v8go.go and include flag set to avoid flag freezing
    - Reorder initialization so allocator is initialized after v8 (required by latest v8)

    * Update V8 static library for macos-11 x86_64

    * Update V8 static library for ubuntu-22.04 x86_64

    * Update V8 static library for macos-11 arm64

    * Update V8 static library for ubuntu-22.04 arm64

    * Update changelog and remove no-longer valid comment

    ---------

    Co-authored-by: jacques-n <[email protected]>

commit 7d843f1
Author: Kyle Maxwell <[email protected]>
Date:   Thu Jan 19 13:17:30 2023 -0800

    v8.Value becomes manually releaseable (rogchap#361)

    * values are manually releaseable

    Co-authored-by: Kyle Maxwell <[email protected]>

commit 1f00b50
Author: Ryan H. Lewis <[email protected]>
Date:   Wed Nov 2 14:15:10 2022 -0600

    adding additional profile values (rogchap#341)

commit fc8b9f1
Author: Ryan H. Lewis <[email protected]>
Date:   Mon Aug 8 16:10:42 2022 -0600

    updating context documentation (rogchap#335)

    Co-authored-by: Ryan H Lewis <[email protected]>

commit da7f43c
Author: Lukas Malkmus <[email protected]>
Date:   Wed Apr 13 16:20:09 2022 +0200

    Fix typo in promise.go (rogchap#310)

    Literally just fixing a typo in `promise.go` that I discovered while strolling through the code.

commit db78170
Author: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Date:   Wed Mar 9 14:29:03 2022 -0800

    Upgrade V8 binaries for 9.7.106.19 version (rogchap#278)

    * Upgrade V8 binaries for 9.7.106.19 version

    * Update V8 static library for ubuntu-18.04 x86_64 (rogchap#292)

    Co-authored-by: genevieve <[email protected]>

    * Update V8 static library for ubuntu-18.04 arm64 (rogchap#293)

    Co-authored-by: genevieve <[email protected]>

    * Update V8 static library for macos-11 x86_64 (rogchap#294)

    Co-authored-by: genevieve <[email protected]>

    * Update V8 static library for macos-11 arm64 (rogchap#295)

    Co-authored-by: genevieve <[email protected]>

    Co-authored-by: dylanahsmith <[email protected]>
    Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
    Co-authored-by: genevieve <[email protected]>

commit 5e91d3d
Author: Dylan Thacker-Smith <[email protected]>
Date:   Wed Jan 12 17:06:50 2022 -0500

    Fix Object.Set with an empty key string (rogchap#276)

commit 1d433f7
Author: Genevieve <[email protected]>
Date:   Wed Jan 12 10:41:04 2022 -0800

    Use length to ensure null chars do not cause early termination of C string copies/reads (rogchap#272)

    * Intro test case for null terminator string

    * unexpected result: expected ss, got sx00s

    * Fix the assertion, add Unicode symbols

    * Pass go string length into v8::String::New to ensure it does not cut off
    at null chars

    * Reuse the existing RtnString type

    * Formatting

    * Update value_test.go

    Co-authored-by: Dylan Thacker-Smith <[email protected]>

    * Table tests for NewValue

    * Use std::string constructor in CopyString(Utf8Value) to keep whole
    string

    * Update changelog

    Co-authored-by: Genevieve L'Esperance <[email protected]>
    Co-authored-by: Maxim Shirshin <[email protected]>
    Co-authored-by: Dylan Thacker-Smith <[email protected]>

commit cb8779b
Author: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Date:   Wed Jan 12 07:51:03 2022 -0800

    Upgrade V8 binaries for 9.7.106.18 version (rogchap#264)

    Co-authored-by: dylanahsmith <[email protected]>
    Co-authored-by: Genevieve <[email protected]>

commit c0dbfd3
Author: Dylan Thacker-Smith <[email protected]>
Date:   Wed Jan 12 09:58:36 2022 -0500

    Remove unnecessary nested Locker and Isolate::Scope use (rogchap#275)

    * Remove unnecessary nested Locker and Isolate::Scope use

    * Make ExceptionError static, since it isn't used outside that file

commit ede7cee
Author: Liu Xiangchao <[email protected]>
Date:   Fri Jan 7 01:33:29 2022 +0800

    modify -fpic to -fPIC (rogchap#263)

commit a0417ad
Author: Dylan Thacker-Smith <[email protected]>
Date:   Wed Dec 22 15:48:45 2021 -0500

    CHANGELOG: Highlight the subtlety of the Function Call breaking change (rogchap#259)

commit 943fcf9
Author: Aditya Sharma <[email protected]>
Date:   Wed Dec 22 09:30:54 2021 -0800

    Build v8 for linux_arm64 (rogchap#223)

    * Build v8 for linux_arm64

    * Update V8 static library for ubuntu-18.04 arm64

    Co-authored-by: epk <[email protected]>

commit e635d48
Author: Genevieve <[email protected]>
Date:   Mon Dec 13 09:28:33 2021 -0800

    Allocate children array with count (rogchap#256)

commit 6e4af34
Author: Roger Chapman <[email protected]>
Date:   Thu Dec 9 10:29:56 2021 +1100

    v0.7.0

commit 762c7a8
Author: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Date:   Wed Dec 8 15:22:43 2021 -0800

    Upgrade V8 binaries for 9.6.180.12 version (rogchap#231)

    * Upgrade V8 binaries for 9.6.180.12 version

    * Add -ldl flag to linux build to fix missing dlsym errors

    * Update V8 static library for macos-11 x86_64 (rogchap#249)

    Co-authored-by: genevieve <[email protected]>

    * Update V8 static library for ubuntu-18.04 x86_64 (rogchap#250)

    Co-authored-by: genevieve <[email protected]>

    * Update V8 static library for macos-11 arm64 (rogchap#251)

    Co-authored-by: genevieve <[email protected]>

    * Update changelog

    Co-authored-by: dylanahsmith <[email protected]>
    Co-authored-by: Genevieve L'Esperance <[email protected]>
    Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
    Co-authored-by: genevieve <[email protected]>

commit a92639e
Author: Genevieve <[email protected]>
Date:   Tue Nov 30 12:59:03 2021 -0800

    Enable intl support and store data in v8 binary (rogchap#242)

    * Enable intl support and store data in v8 binary

    * Update V8 static library for ubuntu-18.04 x86_64

    * Update V8 static library for macos-11 arm64

    * Update changelog

    Co-authored-by: genevieve <[email protected]>

commit f55271d
Author: Genevieve <[email protected]>
Date:   Mon Nov 22 14:50:43 2021 -0800

    Remove Windows support (rogchap#234)

    * Remove Windows binary support

    * Update CHANGELOG.md

    Co-authored-by: Oliver Fuerst <[email protected]>

    * Update CHANGELOG.md

    Co-authored-by: Dylan Thacker-Smith <[email protected]>

    * Update windows section

    * Update README.md

    Co-authored-by: Dylan Thacker-Smith <[email protected]>

    * Update README.md

    Co-authored-by: Dylan Thacker-Smith <[email protected]>

    * Remove windows fossa step

    * Remove deps/windows_x86_64

    Co-authored-by: Oliver Fuerst <[email protected]>
    Co-authored-by: Dylan Thacker-Smith <[email protected]>
    Co-authored-by: Roger Chapman <[email protected]>
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