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

XS code generators, object groups, etc #428

Closed
wants to merge 1 commit into from

Conversation

phoddie
Copy link
Contributor

@phoddie phoddie commented Mar 29, 2024

This is the profile we use for XS with Fuzzilli. It includes code generators to stress unique aspects of the XS runtime. It has two ObjectGroups and several additional build-ins to fuzz Hardened JavaScript features. It also pulls in the RegExpFuzzerfrom V8.

This is the profile we use for XS with Fuzzilli. It includes code generators to stress unique aspects of the XS runtime. It has two ObjectGroups and several additional build-ins to fuzz Hardened JavaScript features. It also pulls in the RegExpFuzzerfrom V8.
// swift run -c release FuzzilliCli --profile=xs --jobs=8 --storagePath=./results --resume --timeout=200 $MODDABLE/build/bin/mac/debug/xst

fileprivate let StressXSGC = CodeGenerator("StressXSGC", inputs: .required(.function())) { b, f in
guard b.type(of: f).Is(.function()) else { return } //@@ where did this come from??
Copy link
Collaborator

Choose a reason for hiding this comment

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

yes, I don't think you need this line here and in the other generators if you have inputs: .required(.function())

b.callFunction(f, withArgs: arguments)
b.unary(.PostInc, index)
let result = b.callFunction(gc, withArgs: [index])
b.buildIfElse(result, ifBody: {
Copy link
Collaborator

Choose a reason for hiding this comment

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

You could build the if like we do it here:

b.buildIf(cond) {
b.build(n: 5)
}

fileprivate let HardenGenerator = CodeGenerator("HardenGenerator", inputs: .required(.object())) { b, obj in
let harden = b.loadBuiltin("harden")

if (Int.random(in: 0...20) < 1) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

you can use probability(0.05) similar to here:


if (Int.random(in: 0...20) < 1) {
let lockdown = b.loadBuiltin("lockdown")
b.callFunction(lockdown, withArgs: [])
Copy link
Collaborator

Choose a reason for hiding this comment

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

No need to add the empty arguments here, we have a default argument for that. So you can call it like this:
b.callFunction(lockdown)

options["loadNowHook"] = loadNowHook;
options["loadHook"] = loadHook;

if (Int.random(in: 0...100) < 50) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

same here, please use probability(0.5)

}
*/

// This template fuzzes the RegExp engine.
Copy link
Collaborator

Choose a reason for hiding this comment

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

I guess these comments are stale here / not relevant, so maybe just remove them?

b.doReturn(resultVar)
}

b.eval("%SetForceSlowPath(false)");
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think this should just fail to parse on XS? so maybe remove these b.eval calls (here, line 245 and line 247) that call our V8 internals?

/// Type of a JavaScript Compartment object.
static let jsCompartment = ILType.object(ofGroup: "Compartment", withProperties: ["globalThis"], withMethods: ["evaluate", "import", "importNow" /* , "module" */])

static let jsCompartmentConstructor = ILType.constructor([.function()] => .jsCompartment) + .object(ofGroup: "CompartmentConstructor", withProperties: ["prototype"], withMethods: [])
Copy link
Collaborator

Choose a reason for hiding this comment

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

The signature for the constructor here does not match the signature you actually call in the CompartmentGenerator.

I think you want it to look something like this?
ILType.constructor([.object(), .object(), .object()] => .jsCompartment] ....
Where the objects are the endowmentsObject, the moduleMapObject and the OptionsObject?


static let jsModuleSource = ILType.object(ofGroup: "ModuleSource", withProperties: ["bindings", "needsImport", "needsImportMeta"])

static let jsModuleSourceConstructor = ILType.constructor([.function()] => .jsModuleSource) + .object(ofGroup: "ModuleSourceConstructor", withProperties: ["prototype"], withMethods: [])
Copy link
Collaborator

Choose a reason for hiding this comment

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

Same here, I think you want it to be:
[.string] => .jsModuleSource
as the argument is a codestring?

"placeholder" : .function([] => .undefined),

// hardened javascript
"Compartment" : .function([] => .jsCompartmentConstructor),
Copy link
Collaborator

Choose a reason for hiding this comment

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

Same here as with the signatures in the object groups, please provide the types that these constructors expect.

@phoddie
Copy link
Contributor Author

phoddie commented Apr 3, 2024

@carl-smith, thanks for the thorough review. I'll try to get all that cleaned up.

FWIW – XSProfile is an admittedly a bit messy. It has evolved over time while I've been learning a (little) bit about Swift and Fuzzilli APIs along the way. Despite the imperfections, it has been incredibly valuable in finding subtle issues in XS. Thanks so much for your work on this project.

@phoddie
Copy link
Contributor Author

phoddie commented Apr 4, 2024

Closing. Replaced by #431.

@phoddie phoddie closed this Apr 4, 2024
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