fix: add buffer-length check in zip_dir_add.c - #440
Open
anupamme wants to merge 252 commits into
Open
Conversation
* fix: prevent JS function to native block leak * fix: validate if isolate is alive
* fix: bridge release adapters * fix: adapter cleanup and thread safety * fix: safeguards on dictionary adapter dealloc
Convert a char* into a JS string (~10x faster than doing NSString.stringWithUTF8String().toString())
* feat: expose inspect and format as the ns:util builtin module
Runtime-provided modules now resolve under the URL-style `ns:` scheme,
ahead of any filesystem or npm resolution, with `node:` compatibility
shims served from the same registry. v1 ships `ns:util` (inspect, format)
and a `node:util` shim re-exporting its members from a distinct, frozen
module object.
Each specifier is one source file and one registry entry: a shim is its
own builtin that reaches the `ns:` module it adapts through a new fixed
wrapper parameter, `require`, which resolves builtin specifiers and
nothing else. Shims therefore compile only when first resolved and own
every bit of Node adaptation, keeping compatibility knowledge out of the
standard modules.
format() is Node's util.format (%s %d %i %f %j %o %O %%, extras appended
space-separated), and console.* routes its arguments through it, so
`console.log("%d apples", 3)` works while unknown and dangling percent
signs stay verbatim.
Resolution is wired into the CommonJS require path, the ES module resolve
callback and the dynamic-import callback; ESM consumption is served by a
per-realm synthetic module. An unknown name in either scheme fails with
`No such built-in module: <specifier>`, which replaces the warn-and-export-
nothing polyfill previously handed to unshimmed `node:` imports. Bare
specifiers are untouched.
docs/ns-builtin-modules.md is the cross-runtime contract both runtimes
implement.
* docs: record the no-source-text-modules decision; js2c rejects non-.js inputs
Builtins stay classic function bodies on both runtimes; cross-builtin
sharing, if ever needed, starts with generation-time bundling. The js2c
guard turns an accidental .mjs into a build error pointing at the spec.
…t#423) Bumps [actions/setup-node](https://github.com/actions/setup-node) from 6.0.0 to 7.0.0. - [Release notes](https://github.com/actions/setup-node/releases) - [Commits](actions/setup-node@2028fbc...8207627) --- updated-dependencies: - dependency-name: actions/setup-node dependency-version: 7.0.0 dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> [skip ci]
Bumps [github/codeql-action/upload-sarif](https://github.com/github/codeql-action) from 4.36.2 to 4.37.3. - [Release notes](https://github.com/github/codeql-action/releases) - [Changelog](https://github.com/github/codeql-action/blob/main/CHANGELOG.md) - [Commits](github/codeql-action@8aad20d...e4fba86) --- updated-dependencies: - dependency-name: github/codeql-action/upload-sarif dependency-version: 4.37.3 dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> [skip ci]
) Bumps [actions/checkout](https://github.com/actions/checkout) from 7.0.0 to 7.0.1. - [Release notes](https://github.com/actions/checkout/releases) - [Changelog](https://github.com/actions/checkout/blob/main/CHANGELOG.md) - [Commits](actions/checkout@9c091bb...3d3c42e) --- updated-dependencies: - dependency-name: actions/checkout dependency-version: 7.0.1 dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> [skip ci]
…ript#426) Bumps [ossf/scorecard-action](https://github.com/ossf/scorecard-action) from 2.4.3 to 2.4.4. - [Release notes](https://github.com/ossf/scorecard-action/releases) - [Changelog](https://github.com/ossf/scorecard-action/blob/main/RELEASE.md) - [Commits](ossf/scorecard-action@4eaacf0...2d11466) --- updated-dependencies: - dependency-name: ossf/scorecard-action dependency-version: 2.4.4 dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> [skip ci]
…ativeScript#427) Bumps [step-security/harden-runner](https://github.com/step-security/harden-runner) from 2.19.4 to 2.20.0. - [Release notes](https://github.com/step-security/harden-runner/releases) - [Commits](step-security/harden-runner@9af89fc...bf7454d) --- updated-dependencies: - dependency-name: step-security/harden-runner dependency-version: 2.20.0 dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> [skip ci]
- V-001 security vulnerability - reject unsafe zip entry names to prevent path traversal (CWE-22)
* A resurrected object can outlive a referenced object's native half, leaving it holding wrappers whose internal field DisposeValue neutered to undefined. Reading that field unchecked as an External fabricates a garbage wrapper pointer — reproduced as a segfault in Interop::SetStructPropertyValue, and the prime suspect for heap corruption discovered later by GC marking. * tns::GetValueOrReport now guards every such read: the Pointer and ExtVector methods, both struct cases of SetStructPropertyValue, the struct property interceptors and GetStructData, the interop.bufferFromData and stringFromCString readers, and the Reference accessors (fixing a pre-existing null-deref in SetValueCallback along the way). * What a guarded touch does is configurable via the new ns:runtime builtin module: setConfig/getConfig with a native key registry. The releasedObjectPolicy key is process-wide, writable from the main isolate only, and defaults to 'report' — the operation no-ops (reads yield undefined, writes are skipped, toString yields a placeholder) and a cancelable releasednativeaccess event fires on globalThis, deduplicated per released object, carrying the touch site's stack and the API surface. The debug console.warn is the event's default action, so a listener that forwards the report can preventDefault() it. 'throw' raises a catchable ReferenceError at the touch site instead. * The specs cover both policies in one run, plus the ns:runtime surface (defaults, validation, singleton). See docs/ns-builtin-modules.md and the Known hazard section of docs/knowledge/v8-resurrecting-finalizers.md.
* `npx lint-staged || echo ...` converted every failure — missing tools included — into a warning: with no node_modules the eslint task died with ENOENT, lint-staged killed the sibling clang-format task, and the commit went through unvalidated while the message blamed clang-format. * Preflight node_modules with an accurate instruction, run lint-staged with --no-install so only the pinned version can run, and let its exit code block the commit. Explicit bypass stays available via --no-verify.
* Whole-file clang-format -i churns legacy files that predate the format standard: the first commit touching one mixed real changes with historical reformatting. git clang-format --staged --diff judges only the staged hunks, never mutates the tree (safe with git add -p), and fails with the exact fix command. * The extension list is load-bearing: git-clang-format's default set includes .js, which eslint owns via lint-staged. * Preflight git-clang-format alongside node_modules; both ship with any clang-format install (brew included).
…kip ci] * Replaces the check-and-fail gate: committing needed a manual formatting step. Running git clang-format HEAD inside lint-staged formats exactly the staged hunks (the working tree equals the staged content there) and lint-staged re-stages the result — same commit-and-forget flow as the old clang-format -i task, without reformatting the untouched remainder of legacy files, and safe with partial staging. * The wrapper maps git-clang-format's exit 1 (files were reformatted) to success; real errors die() with exit 2 and still block.
Ambient declare-module types for ns:runtime and ns:util, one .d.ts per module behind a referencing index (the package.json types entry point), mirroring @types/node's layout. setConfig/getConfig are generic over an augmentable RuntimeConfig key map so keys registered by other runtimes or newer versions still compile, degrading to unknown. node:util is deliberately undeclared to avoid clashing with @types/node. The npm build scripts now copy types/ into the package, and the ns module tests assert the exact export sets so the runtime surface, the declarations, and docs/ns-builtin-modules.md move together.
detail is cloned once at entry creation, so entries hold snapshots and an uncloneable detail throws the DataCloneError-named error. The clone goes through structuredClone captured at builtin init, with an identity fallback that keeps performance.js portable to a runtime shipping the Performance API before structuredClone. Removes the by-reference deviation from the docs and bumps the shared tests to the suite that asserts both contracts.
…Script#433) Two things stopped Mac Catalyst builds from producing usable metadata: - `MACCATALYST_DEPLOYMENT_TARGET` is not exported by Xcode unless the project sets it explicitly, so the script died with `KeyError` before generating anything. Fall back to `IPHONEOS_DEPLOYMENT_TARGET`. - UIKit and the other iOS-flavoured frameworks live under `System/iOSSupport` in the macOS SDK. Xcode passes those to the real compile with `-iframework`, which never reaches `FRAMEWORK_SEARCH_PATHS`, so the generator parsed the macOS SDK with no UIKit in scope and emitted metadata without it. Every UIKit class then failed at runtime with `ReferenceError: UIDevice is not defined`. Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
- formatMapLike takes the entry count from the captured Map/Set size accessors and stops iterating at MAX_ENTRIES, so rendering a large collection no longer walks every entry to produce a bounded line. - inspect.js states Proxy as the exception to the no-user-code rule, and guards the symbol-key descriptor against a trap dropping the key between getOwnPropertySymbols and getOwnPropertyDescriptor. - The Invalid Date check uses the NumberIsNaN primordial rather than a self compare. - InspectTests covers toJSON non-invocation and the bounded Map/Set path. Ports the Android-side review fixes (mirrors the CodeRabbit findings on NativeScript/android#1991).
The dynamic-import builtin failure path rejected the resolver while the exception caught from GetModule was still pending on the isolate; V8 API calls with a pending exception are unsafe. The sibling non-builtin path already resets before rejecting. Same ordering fix as the Android runtime (review finding on NativeScript/android#1992).
WebIDL converts null for a (DOMString or PerformanceMeasureOptions) union to an empty dictionary, so measure(name, null) means "no options" rather than the mark name "null", which is what it resolved to and threw on. A null endMark keeps throwing: that parameter is a plain DOMString, neither a union nor nullable, so null stringifies per WebIDL.
The 10ms reconstruction budget flaked on contended CI hosts (~20ms scheduler stalls between the clock reads); the shared suite now takes the minimum offset over ten samples against a 250ms bound.
The spec measured from "the-start", which it never created, so the SyntaxError it asserts could have come from that missing mark rather than from the null end mark it is about.
…(CFRunLoop timer/source) (NativeScript#439)
Automated security fix generated by OrbisAI Security
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Contributor
|
@anupamme not sure why your PR targets dev by default - |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
14 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fix critical severity security issue in
TKLiveSync/libzip/zip_dir_add.c.Vulnerability
V-001TKLiveSync/libzip/zip_dir_add.c:65Description: The zip_dir_add function uses the unsafe strcpy() function to copy a filename into a dynamically allocated buffer without explicit bounds checking. Although the buffer size is calculated based on strlen(name), strcpy is inherently unsafe and can lead to buffer overflow if there are discrepancies between the calculated length and actual copy operation.
Evidence
Exploitation scenario: An attacker provides a malicious filename to the archive addition function.
Scanner confirmation: multi_agent_ai rule
V-001flagged this pattern.Production code: This file is in the production codebase, not test-only code.
Threat Model Context
This is a Node.js library - vulnerabilities affect downstream consumers who use this package.
Changes
TKLiveSync/libzip/zip_dir_add.cBehavior Preservation
The change is scoped to 1 file on the vulnerable path; it only tightens handling of untrusted input and leaves valid inputs unaffected.
Automated security fix by OrbisAI Security