Conversation
github-actions
Bot
added
the
dangerous
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6bb008acfe
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| exit 1 | ||
| fi | ||
|
|
||
| cp "$filename" "$tempfile" |
There was a problem hiding this comment.
Because /challenge/run is installed setuid-root by common/.init, the -r check happens under the real UID but the subsequent cp runs with effective root. A user can race by swapping a symlink after the readability check and before this cp, causing root to copy a protected file (e.g., /flag) into the temp file that is then chmod 644 and readable in /tmp before removal. This bypasses the intended exploit path and leaks the flag; consider opening the file once (e.g., exec <"$filename" with O_NOFOLLOW) and copying from that fd, or dropping privileges before the copy.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Pull request overview
This pull request ports V8 exploitation challenges from a previous repository to this one, creating a module with 9 progressive V8 JavaScript engine exploitation challenges. The challenges are designed to teach browser exploitation by introducing intentional vulnerabilities through patches to the V8 engine.
Changes:
- Added module configuration with 9 levels of V8 exploitation challenges
- Created common infrastructure (Dockerfile, build scripts, catflag utility, run wrapper)
- Added 9 challenge levels, each with custom V8 patches introducing exploitable vulnerabilities
Reviewed changes
Copilot reviewed 81 out of 83 changed files in this pull request and generated 18 comments.
Show a summary per file
| File | Description |
|---|---|
| challenges/v8-exploitation/module.yml | Module configuration defining 9 challenge levels with visibility settings and reference materials |
| challenges/v8-exploitation/common/* | Shared infrastructure including V8 build configuration, Docker setup, and execution wrapper |
| challenges/v8-exploitation/level-1/* | Challenge with ArrayRun builtin that executes doubles as shellcode |
| challenges/v8-exploitation/level-2/* | Challenge with arbitrary read/write primitives exposed via GetAddressOf, ArbRead32, ArbWrite32 |
| challenges/v8-exploitation/level-3/* | Challenge with GetAddressOf and GetFakeObject primitives for heap manipulation |
| challenges/v8-exploitation/level-4/* | Challenge with ArrayPrototypeSetLength builtin for array length manipulation |
| challenges/v8-exploitation/level-5/* | Challenge with ArrayOffByOne builtin for out-of-bounds access |
| challenges/v8-exploitation/level-6/* | Challenge with ArrayFunctionMap builtin vulnerable to side effects during iteration |
| challenges/v8-exploitation/level-7/* | Challenge with disabled CheckMaps deoptimization for type confusion |
| challenges/v8-exploitation/level-8/* | Challenge with weakened bounds check optimization |
| challenges/v8-exploitation/level-9/* | Challenge with V8 sandbox enabled and memory corruption API exposed |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| + case Builtin::kArrayRun: | ||
| + return Type::Receiver(); |
There was a problem hiding this comment.
Similar indentation inconsistency with tab-prefixed case statement.
| + case Builtin::kArrayRun: | |
| + return Type::Receiver(); | |
| + case Builtin::kArrayRun: | |
| + return Type::Receiver(); |
| #!/bin/bash -ex | ||
| echo 'console.log("ok");' > /tmp/test.js | ||
| /challenge/d8 /tmp/test.js 2>&1 | grep -q ok |
There was a problem hiding this comment.
Only level-1 has private tests (tests_private directory), while levels 2-9 are missing private tests entirely. Based on the repository pattern where other challenges have both public and private tests, this appears to be incomplete. Private tests typically verify that the challenges can actually be solved and that the exploits work as intended.
| Author: [sinamhdv](https://github.com/sinamhdv) | ||
|
|
||
| visibility: | ||
| start: "2024-08-01T17:00:00-07:00" |
There was a problem hiding this comment.
The visibility start date is set to "2024-08-01T17:00:00-07:00", which is in the past. The current date is February 2, 2026. This appears to be intentional since this is described as "porting" content from the "2024 fall Quarterly Quiz", but it's worth confirming this is the intended start date for this repository.
| @@ -0,0 +1,51 @@ | |||
| name: V8 Exploitation | |||
| description: | | |||
| Welcome to the 2024 fall Quarterly Quiz, and to the world of browser exploitation! In this set of challenges, you can explore Chromium's Javascript engine, V8. | |||
There was a problem hiding this comment.
The module description references "2024 fall Quarterly Quiz" but this content is being ported in 2026. Consider updating the description to clarify whether this is archived content or if it should reference the current timeframe.
| "src/sandbox/external-pointer-table.h", | ||
| + "src/sandbox/testing.cc", |
There was a problem hiding this comment.
There is inconsistent indentation in the patch. Lines 8-9 use tabs while most other lines use spaces. While this is within a patch file (which may preserve the original V8 codebase's formatting), it's worth noting for consistency.
| + } | ||
| + | ||
| + // HeapObjects must be allocated inside the pointer compression cage so their | ||
| + // address relative to the start of the sandbox can be obtained simply by | ||
| + // taking the lowest 32 bits of the absolute address. | ||
| + uint32_t address = static_cast<uint32_t>(HeapObject::cast(*arg).address()); | ||
| + args.GetReturnValue().Set(v8::Integer::NewFromUnsigned(isolate, address)); | ||
| +} | ||
| + | ||
| +// Sandbox.getSizeOf(object) -> Number | ||
| +void SandboxGetSizeOf(const v8::FunctionCallbackInfo<v8::Value>& args) { | ||
| + v8::Isolate* isolate = args.GetIsolate(); | ||
| + | ||
| + if (args.Length() == 0) { | ||
| + isolate->ThrowError("First argument must be provided"); | ||
| + return; | ||
| + } | ||
| + | ||
| + Handle<Object> arg = Utils::OpenHandle(*args[0]); | ||
| + if (!arg->IsHeapObject()) { | ||
| + isolate->ThrowError("First argument must be a HeapObject"); | ||
| + return; | ||
| + } | ||
| + | ||
| + int size = HeapObject::cast(*arg).Size(); | ||
| + args.GetReturnValue().Set(v8::Integer::New(isolate, size)); | ||
| +} | ||
| + | ||
| +Handle<FunctionTemplateInfo> NewFunctionTemplate( | ||
| + Isolate* isolate, FunctionCallback func, | ||
| + ConstructorBehavior constructor_behavior) { | ||
| + // Use the API functions here as they are more convenient to use. | ||
| + v8::Isolate* api_isolate = reinterpret_cast<v8::Isolate*>(isolate); | ||
| + Local<FunctionTemplate> function_template = | ||
| + FunctionTemplate::New(api_isolate, func, {}, {}, 0, constructor_behavior, | ||
| + SideEffectType::kHasSideEffect); | ||
| + return v8::Utils::OpenHandle(*function_template); | ||
| +} | ||
| + | ||
| +Handle<JSFunction> CreateFunc(Isolate* isolate, FunctionCallback func, | ||
| + Handle<String> name, bool is_constructor) { | ||
| + ConstructorBehavior constructor_behavior = is_constructor | ||
| + ? ConstructorBehavior::kAllow | ||
| + : ConstructorBehavior::kThrow; | ||
| + Handle<FunctionTemplateInfo> function_template = | ||
| + NewFunctionTemplate(isolate, func, constructor_behavior); | ||
| + return ApiNatives::InstantiateFunction(function_template, name) | ||
| + .ToHandleChecked(); | ||
| +} | ||
| + | ||
| +void InstallFunc(Isolate* isolate, Handle<JSObject> holder, | ||
| + FunctionCallback func, const char* name, int num_parameters, | ||
| + bool is_constructor) { | ||
| + Factory* factory = isolate->factory(); | ||
| + Handle<String> function_name = factory->NewStringFromAsciiChecked(name); | ||
| + Handle<JSFunction> function = | ||
| + CreateFunc(isolate, func, function_name, is_constructor); | ||
| + function->shared().set_length(num_parameters); | ||
| + JSObject::AddProperty(isolate, holder, function_name, function, NONE); | ||
| +} | ||
| + | ||
| +void InstallGetter(Isolate* isolate, Handle<JSObject> object, | ||
| + FunctionCallback func, const char* name) { | ||
| + Factory* factory = isolate->factory(); | ||
| + Handle<String> property_name = factory->NewStringFromAsciiChecked(name); | ||
| + Handle<JSFunction> getter = CreateFunc(isolate, func, property_name, false); | ||
| + Handle<Object> setter = factory->null_value(); | ||
| + JSObject::DefineAccessor(object, property_name, getter, setter, FROZEN); | ||
| +} | ||
| + | ||
| +void InstallFunction(Isolate* isolate, Handle<JSObject> holder, | ||
| + FunctionCallback func, const char* name, | ||
| + int num_parameters) { | ||
| + InstallFunc(isolate, holder, func, name, num_parameters, false); | ||
| +} | ||
| + | ||
| +void InstallConstructor(Isolate* isolate, Handle<JSObject> holder, | ||
| + FunctionCallback func, const char* name, | ||
| + int num_parameters) { | ||
| + InstallFunc(isolate, holder, func, name, num_parameters, true); | ||
| +} | ||
| + | ||
| +} // namespace | ||
| + | ||
| +// static | ||
| +void MemoryCorruptionApi::Install(Isolate* isolate) { | ||
| + CHECK(GetProcessWideSandbox()->is_initialized()); | ||
| + | ||
| + Factory* factory = isolate->factory(); | ||
| + | ||
| + // Create the special Sandbox object that provides read/write access to the | ||
| + // sandbox address space alongside other miscellaneous functionality. | ||
| + Handle<JSObject> sandbox = | ||
| + factory->NewJSObject(isolate->object_function(), AllocationType::kOld); | ||
| + | ||
| + InstallGetter(isolate, sandbox, SandboxGetByteLength, "byteLength"); | ||
| + InstallConstructor(isolate, sandbox, SandboxMemoryView, "MemoryView", 2); | ||
| + InstallFunction(isolate, sandbox, SandboxGetAddressOf, "getAddressOf", 1); | ||
| + InstallFunction(isolate, sandbox, SandboxGetSizeOf, "getSizeOf", 1); | ||
| + |
There was a problem hiding this comment.
The MemoryCorruptionApi exposes a Sandbox object into the JS global scope that provides byteLength, a MemoryView constructor directly wrapping arbitrary slices of the sandbox’s address space in JSArrayBuffers, and helpers like getAddressOf/getSizeOf for heap objects.
This effectively hands untrusted JavaScript direct, mutable access to the sandbox’s raw memory and internal object layout, completely breaking sandbox guarantees and enabling arbitrary memory read/write and native code execution inside the V8 process.
Such low-level inspection and memory access APIs must not be exposed to untrusted code; if needed for testing, they should be compiled out or strictly gated so they cannot be enabled in deployed runtimes.
| rm "$tempfile" | ||
| } | ||
|
|
||
| su -s /bin/bash -c "$(declare -f run_challenge); run_challenge \"$filename\"" nobody |
There was a problem hiding this comment.
The run wrapper constructs a shell command for su -s /bin/bash -c using the untrusted filename inside a double-quoted string (run_challenge "$filename"), which is expanded by the outer shell and then re-parsed by the inner bash -c. If filename contains shell metacharacters (e.g., embedded quotes and semicolons) and the attacker can create such a file, they can break out of the intended run_challenge invocation and inject additional commands executed by bash as user nobody. To avoid this, avoid passing untrusted data through shell command strings at all (e.g., use su/bash argument arrays or carefully escape/quote filename so that it cannot alter the command structure).
| su -s /bin/bash -c "$(declare -f run_challenge); run_challenge \"$filename\"" nobody | |
| func_def="$(declare -f run_challenge)" | |
| cmd="$func_def; run_challenge \"\$1\"" | |
| su -s /bin/bash nobody -c "$cmd" -- "$filename" |
|
|
||
| +void Shell::GetAddressOf(const v8::FunctionCallbackInfo<v8::Value>& info) { | ||
| + v8::Isolate* isolate = info.GetIsolate(); | ||
| + | ||
| + if (info.Length() == 0) { | ||
| + isolate->ThrowError("First argument must be provided"); | ||
| + return; | ||
| + } | ||
| + | ||
| + internal::Handle<internal::Object> arg = Utils::OpenHandle(*info[0]); | ||
| + if (!IsHeapObject(*arg)) { | ||
| + isolate->ThrowError("First argument must be a HeapObject"); | ||
| + return; | ||
| + } | ||
| + internal::Tagged<internal::HeapObject> obj = internal::Cast<internal::HeapObject>(*arg); | ||
| + | ||
| + uint32_t address = static_cast<uint32_t>(obj->address()); | ||
| + info.GetReturnValue().Set(v8::Integer::NewFromUnsigned(isolate, address)); | ||
| +} | ||
| + | ||
| +void Shell::GetFakeObject(const v8::FunctionCallbackInfo<v8::Value>& info) { | ||
| + v8::Isolate *isolate = info.GetIsolate(); | ||
| + Local<v8::Context> context = isolate->GetCurrentContext(); | ||
| + | ||
| + if (info.Length() != 1) { | ||
| + isolate->ThrowError("Need exactly one argument"); | ||
| + return; | ||
| + } | ||
| + | ||
| + Local<v8::Uint32> arg; | ||
| + if (!info[0]->ToUint32(context).ToLocal(&arg)) { | ||
| + isolate->ThrowError("Argument must be a number"); | ||
| + return; | ||
| + } | ||
| + | ||
| + uint32_t addr = arg->Value(); | ||
| + | ||
| + internal::PtrComprCageBase cage_base = internal::GetPtrComprCageBase(); | ||
| + internal::Address base_addr = internal::V8HeapCompressionScheme::GetPtrComprCageBaseAddress(cage_base); | ||
| + uint64_t full_addr = base_addr + (uint64_t)addr; | ||
| + | ||
| + internal::Tagged<internal::HeapObject> obj = internal::HeapObject::FromAddress(full_addr); | ||
| + internal::Isolate *i_isolate = reinterpret_cast<internal::Isolate*>(isolate); | ||
| + internal::Handle<internal::Object> obj_handle(obj, i_isolate); | ||
| + info.GetReturnValue().Set(ToApiHandle<v8::Value>(obj_handle)); |
There was a problem hiding this comment.
The GetAddressOf and GetFakeObject helpers added to Shell give JavaScript the ability to obtain raw heap object addresses and synthesize new JS values from arbitrary addresses by reconstructing compressed pointers. This enables direct type confusion and arbitrary memory access from JS (by forging fake objects pointing into controlled memory), breaking V8’s isolation between JS and native memory and enabling native code execution and sensitive data theft from the engine process. These APIs should not be exposed to untrusted scripts and, if needed for testing, should be compiled out or gated so they are unavailable in production challenge/runtime builds.
|
|
||
| +BUILTIN(ArrayOffByOne) { | ||
| + HandleScope scope(isolate); | ||
| + Factory *factory = isolate->factory(); | ||
| + Handle<Object> receiver = args.receiver(); | ||
| + | ||
| + if (!IsJSArray(*receiver) || !HasOnlySimpleReceiverElements(isolate, Cast<JSArray>(*receiver))) { | ||
| + THROW_NEW_ERROR_RETURN_FAILURE(isolate, NewTypeError(MessageTemplate::kPlaceholderOnly, | ||
| + factory->NewStringFromAsciiChecked("Nope"))); | ||
| + } | ||
| + | ||
| + Handle<JSArray> array = Cast<JSArray>(receiver); | ||
| + | ||
| + ElementsKind kind = array->GetElementsKind(); | ||
| + | ||
| + if (kind != PACKED_DOUBLE_ELEMENTS) { | ||
| + THROW_NEW_ERROR_RETURN_FAILURE(isolate, NewTypeError(MessageTemplate::kPlaceholderOnly, | ||
| + factory->NewStringFromAsciiChecked("Need an array of double numbers"))); | ||
| + } | ||
| + | ||
| + if (args.length() > 2) { | ||
| + THROW_NEW_ERROR_RETURN_FAILURE(isolate, NewTypeError(MessageTemplate::kPlaceholderOnly, | ||
| + factory->NewStringFromAsciiChecked("Too many arguments"))); | ||
| + } | ||
| + | ||
| + Handle<FixedDoubleArray> elements(Cast<FixedDoubleArray>(array->elements()), isolate); | ||
| + uint32_t len = static_cast<uint32_t>(Object::NumberValue(array->length())); | ||
| + if (args.length() == 1) { // read mode | ||
| + return *(isolate->factory()->NewNumber(elements->get_scalar(len))); | ||
| + } else { // write mode | ||
| + Handle<Object> value = args.at(1); | ||
| + if (!IsNumber(*value)) { | ||
| + THROW_NEW_ERROR_RETURN_FAILURE(isolate, NewTypeError(MessageTemplate::kPlaceholderOnly, | ||
| + factory->NewStringFromAsciiChecked("Need a number argument"))); | ||
| + } | ||
| + double num = static_cast<double>(Object::NumberValue(*value)); | ||
| + elements->set(len, num); | ||
| + return ReadOnlyRoots(isolate).undefined_value(); |
There was a problem hiding this comment.
The ArrayOffByOne builtin deliberately reads and writes element index len of a FixedDoubleArray (elements->get_scalar(len) / elements->set(len, ...)), even though valid indices are 0..len-1. This is a direct out-of-bounds read/write primitive from JavaScript into the array’s backing store, allowing attackers to corrupt adjacent heap memory or read unintended data, which is a classic path to reliable native code execution in the V8 process. Any array accessor must enforce strict bounds checks and never allow JS to access elements[len] without a corresponding allocation guaranteeing that slot exists.
| +++ b/src/compiler/turboshaft/machine-lowering-reducer-inl.h | ||
| @@ -2740,7 +2740,7 @@ class MachineLoweringReducer : public Next { | ||
| const ZoneRefSet<Map>& maps, CheckMapsFlags flags, | ||
| const FeedbackSource& feedback) { | ||
| if (maps.is_empty()) { | ||
| - __ Deoptimize(frame_state, DeoptimizeReason::kWrongMap, feedback); | ||
| + //__ Deoptimize(frame_state, DeoptimizeReason::kWrongMap, feedback); | ||
| return {}; | ||
| } | ||
|
|
||
| @@ -2749,14 +2749,14 @@ class MachineLoweringReducer : public Next { | ||
| IF_NOT (LIKELY(CompareMapAgainstMultipleMaps(heap_object_map, maps))) { | ||
| // Reloading the map slightly reduces register pressure, and we are on a | ||
| // slow path here anyway. | ||
| - MigrateInstanceOrDeopt(heap_object, __ LoadMapField(heap_object), | ||
| - frame_state, feedback); | ||
| - __ DeoptimizeIfNot(__ CompareMaps(heap_object, maps), frame_state, | ||
| - DeoptimizeReason::kWrongMap, feedback); | ||
| + //MigrateInstanceOrDeopt(heap_object, __ LoadMapField(heap_object), | ||
| + // frame_state, feedback); | ||
| + //__ DeoptimizeIfNot(__ CompareMaps(heap_object, maps), frame_state, | ||
| + // DeoptimizeReason::kWrongMap, feedback); | ||
| } | ||
| } else { | ||
| - __ DeoptimizeIfNot(__ CompareMaps(heap_object, maps), frame_state, | ||
| - DeoptimizeReason::kWrongMap, feedback); | ||
| + //__ DeoptimizeIfNot(__ CompareMaps(heap_object, maps), frame_state, | ||
| + // DeoptimizeReason::kWrongMap, feedback); |
There was a problem hiding this comment.
The changes in MachineLoweringReducer::CheckMaps comment out all deoptimization and map migration paths when a heap object’s map does not match the expected set, effectively turning failed map checks into silent success. This breaks Turboshaft’s key type and shape assumptions, allowing just-in-time compiled code to treat objects with incompatible layouts as if they had the expected map, which is a classic type confusion primitive that can lead to arbitrary memory reads/writes and native code execution from crafted JavaScript. Map checks in optimized code are fundamental safety guards and must never be disabled or converted into no-ops for untrusted inputs.
8526647 to
50303d5
Compare
4d306d7 to
122e843
Compare
First challenge in the Maglev compiler exploitation submodule. Students observe V8's multi-tier JIT pipeline by triggering Maglev and TurboFan independently, reading %GetOptimizationStatus() bits, and passing them to WinMaglevObserved(). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Bridge challenge exploiting Maglev's BuildAllocateFastObject missing double element initialization. Students trigger Maglev compilation of a function with a double array literal and observe uninitialized element values. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Prove that Maglev's allocation folding incorrectly spans the super() call boundary — the root cause of CVE-2024-0517. The WinAllocationFolding helper checks that objects allocated in a derived constructor's body are folded (adjacent in memory) with the super() allocation due to a missing ClearCurrentRawAllocation() call. No maglev-graph-builder.cc patch needed since the bug exists natively in V8 12.0. Also fix .init.j2 in maglev-warmup and maglev-1 to use 4755 permissions on catflag (world-executable with SUID) for reliable test execution. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…-2024-4947 and reorder module.yml Add remaining Maglev challenges completing the 7-challenge submodule: - maglev-3: GC-triggered OOB bridge with GetAddressOf helper - cve-2024-0517: full CVE exploitation on educational V8 revision - cve-2024-0517-real: CVE on original vulnerable V8 12.0 (no helpers) - cve-2024-4947: Lazarus Group ITW 0-day, module namespace AccessInfo type confusion on V8 12.4 Move Maglev Compiler Exploitation section in module.yml to its correct position between "JIT Bugs on the V8 Heap" and "Garbage Collection Internals" per MAGLEV_MODULE_PLAN.md. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Thorough review of all 57 challenges covering inventory, difficulty progression, learning gaps, quality assessment, and recommendations. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This is just porting to this repo. All credit goes to @sinamhdv.