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

Issue running stealth_extension #1081

Open
jackiecan opened this issue Jan 10, 2024 · 6 comments · May be fixed by #1037
Open

Issue running stealth_extension #1081

jackiecan opened this issue Jan 10, 2024 · 6 comments · May be fixed by #1037

Comments

@jackiecan
Copy link

Hi,

I'm currently working on setting up the Stealth Extension by @bkrumnow on a freshly installed Ubuntu 18.04 system by running the stealth_extension branch. However, I'm encountering errors when npm ci is run in /scripts/build-extension.sh
The following error messages appearing regarding the instrument.ts and stealth.ts files during the execution of the > build:main command (see below)

Full console Log and Debug log of run of install.sh: console_log_install.txt, 2024-01-10T09_47_20_547Z-debug-0.log

Based on the information from Pull Request #1037, it appears that the extension might not be stable at the moment? I'm wondering if you have any insights into why these errors are occurring and if there are any steps I can take to successfully use the extension.

I appreciate your assistance in advance! Thanks for maintaining the tool!

Best Regards :)

ERROR:

src/stealth/instrument.ts:259:47 - error TS2339: Property 'logFunctionsAsStrings' does not exist on type '{ object: string; instrumentedName: string; depth: number; logSettings: { propertiesToInstrument: any[]; nonExistingPropertiesToInstrument: any[]; excludedProperties: string[]; overwrittenProperties: any[]; ... 5 more ...; depth: number; }; } | { ...; } | { ...; } | { ...; }'.
  Property 'logFunctionsAsStrings' does not exist on type '{ object: string; instrumentedName: string; depth: number; logSettings: { propertiesToInstrument: any[]; nonExistingPropertiesToInstrument: any[]; excludedProperties: string[]; overwrittenProperties: any[]; ... 5 more ...; depth: number; }; }'.

259     value: serializeObject(value, logSettings.logFunctionsAsStrings),
                                                  ~~~~~~~~~~~~~~~~~~~~~

#### some more errors

src/stealth/stealth.ts:86:24 - error TS2345: Argument of type '{ context: any; wrappedWindow: any; changeWindowProperty: (object: any, name: any, type: any, changed: any) => void; singleCallback: any; allCallback: any; }' is not assignable to parameter of type '{ context: any; wrappedWindow: any; changeWindowProperty: any; observe: any; allCallback: any; }'.
  Property 'observe' is missing in type '{ context: any; wrappedWindow: any; changeWindowProperty: (object: any, name: any, type: any, changed: any) => void; singleCallback: any; allCallback: any; }' but required in type '{ context: any; wrappedWindow: any; changeWindowProperty: any; observe: any; allCallback: any; }'.

86   protectDocumentWrite(api);
                          ~~~

#### some more errors

Errors  Files
    11  src/stealth/instrument.ts:259
     8  src/stealth/stealth.ts:56
npm verb exit 2
npm verb code 2
npm verb exit 2
npm verb code 2
npm verb stack Error: command failed
npm verb stack     at ChildProcess.<anonymous> (/home/vagrant/miniforge3/envs/openwpm/lib/node_modules/npm/node_modules/@npmcli/promise-spawn/lib/index.js:53:27)
npm verb stack     at ChildProcess.emit (node:events:514:28)
npm verb stack     at maybeClose (node:internal/child_process:1105:16)
npm verb stack     at ChildProcess._handle.onexit (node:internal/child_process:305:5)
npm verb cwd /home/vagrant/openwpm/OpenWPM/Extension
npm verb Linux 5.4.0-169-generic
npm verb node v20.6.1
npm verb npm  v9.8.1
npm ERR! code 2
npm ERR! path /home/vagrant/openwpm/OpenWPM/Extension
npm ERR! command failed
npm ERR! command sh -c npm run build && npm run test
npm verb exit 2
npm verb code 2
@vringar
Copy link
Contributor

vringar commented Jan 11, 2024

Hey @jackiecan,

I haven't had the time to push this PR any further as the semester began and likely won't get to any more work on the PR until early March.

From what I recall, I'd expect the extension to be functional, if you could get it to compile/pass the TypeScript checker (tsc) which is giving you these errors. Depending on your level of familiarity with TypeScript, you might be able to work yourself through the type errors. (If you are proficient, you might be able to continue the work I did and model the added global functions, otherwise try adding more as any casts until it compiles.)

Another reason that this PR isn't merged yet is a lack of tests. I'd like to make sure that the benefits claimed in the paper (primarily around decreased detection surface) stay working on future Firefox releases.

However, in any case you should try and rebase the work on the current master, as you'll be running an outdated Firefox otherwise.

@vringar
Copy link
Contributor

vringar commented Jan 25, 2024

@jackiecan I fixed the tsc issues now it's just a whole bunch of eslint stuff. So if you have some time to spend on this, you could easily get this into a working state. I also rebased the PR against the current master

@jackiecan
Copy link
Author

Hi, thank you so much for keeping up with this! I'm sorry that I did't respond before, I thought I answered, but it seems like I never sent that message.
I tried to run it, encountered two little problems, but was able to fix them. First some Type-Checking Failed. I think this is automatically checked before a crawl is started, even when the js_instrument isn't used.

jsonschema.exceptions.ValidationError: 'name' is not of type 'object'

Failed validating 'type' in schema['items']['properties']['logSettings']['properties']['propertiesToInstrument']['items']:
    {'properties': {'depth': {'type': 'integer'},
                    'propertyNames': {'items': {'type': 'string'},
                                      'type': 'array'}},
     'type': 'object'}

On instance[11]['logSettings']['propertiesToInstrument'][0]:
    'name'

Not shure if this is legit, but I was able to fix this by changing the propertiesToInstrument for logSettings in js_instrument_settings.schema.json to

"propertiesToInstrument": {
  "items": {
    "type": "string"
}}

because in fingerprinting.json it's just "propertiesToInstrument": ["name", "localStorage", "sessionStorage"].
This is also how it is in the current master.

Then the program "crashed" when the following was called in deploy_firefox.py.

    fo.binary = FirefoxBinary(
        firefox_path=firefox_binary_path, log_file=open(interceptor.fifo, "w")
    )
-------------------------------
Traceback (most recent call last):
  File "/home/vagrant/Desktop/OpenWPM/openwpm/browser_manager.py", line 735, in run
    driver, browser_profile_path, display = deploy_firefox.deploy_firefox(
                                            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/vagrant/Desktop/OpenWPM/openwpm/deploy_browsers/deploy_firefox.py", line 152, in deploy_firefox
    executable_path=geckodriver_path, log_output=open(interceptor.fifo, "w")
                                                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^

It appears that the fifo wasn't present anymore when called for the second time.
fo.binary = FirefoxBinary(...) also seems to be deprecated, so I changed it to fo.binary_location = firefox_binary_path which then worked.

Now there are only some eslint things left. But the Extension runs fine.

@vringar
Copy link
Contributor

vringar commented Feb 7, 2024

Thanks for reporting both issues.
The second one was a race condition that also affected the master branch. I'm honestly shocked that our testing never caught this.

The ESLint issues should be fixed now

@bkrumnow
Copy link

bkrumnow commented Feb 8, 2024

Hi @vringar,
Yes, there is one tradeoff: prototype pollution. The stealth_extension avoids prototype pollution on the cost of tracing origin. That means if you have multiple objects that share a prototype and a function is called from this prototype, you cannot determine the object from which the function was called.

This issue may be irrelevant for many measurements; however, others may need this information. Supporting both variants (stealth_extension with pollution and without it) should be feasible. I am happy to point you to the code fragments if required. If you are not in a hurry, I can make the necessary changes myself.

You can access the tooling we used here:

I would also like to check the FP surface before you merge it. Just let me know when it is ready.

@vringar
Copy link
Contributor

vringar commented Feb 8, 2024

Hey @bkrumnow,
Thanks for responding so quickly and so thoroughly.
I am in absolutely no rush to get this merged. I just got a little excited that it finally type checked.

I'd really appreciate it if you could make it support both modes as the WebExtension really isn't a strong area of the code base for me.

Thanks for also linking the detectors. I'll turn them into tests when I find the time.

I'll definitely let you know when this PR is ready from my POV, so you can do the final testing.

@vringar vringar linked a pull request Feb 8, 2024 that will close this issue
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 a pull request may close this issue.

3 participants