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

Implement functionality proposed in JavaScript Zero #39

Closed
wants to merge 19 commits into from

Conversation

PeetHornak
Copy link

Paper: https://misc0110.net/web/files/jszero.pdf
Proof of concept: https://github.com/IAIK/ChromeZero
Fixes:

Now works on Chrome
Parse sub domains while setting level
Makefile works on MacOS

Functionality:

Slow WebWorkers messages
WebWorker polyfill, disables true parallelism
Slow SharedArrayBuffer messages
Disable SharedArrayBuffer
Wrap DataView and TypedArrays - randomly memory access, random mapping of indexes to memory, preloading in constructor,
Random noise in High resolution timestamps

Buffer ASLR was not implemented even though it was proposed in paper.

@PeetHornak PeetHornak changed the base branch from master to rewrite May 12, 2020 17:08
@polcak
Copy link
Owner

polcak commented May 13, 2020

409f3dd split into d880889 and aa523a1. Pushed and fixed #35

polcak added a commit that referenced this pull request May 13, 2020
See also #39 and commit e0c1142
@polcak
Copy link
Owner

polcak commented May 13, 2020

Can 70199a9 replace e0c1142?

Note that neither 70199a9 nor e0c1142 is in rewrite branch, yet.

@polcak
Copy link
Owner

polcak commented May 13, 2020

b240c61 pushed as afe712c

@PeetHornak
Copy link
Author

Can 70199a9 replace e0c1142?

Note that neither 70199a9 nor e0c1142 is in rewrite branch, yet.

From my point of view, it can be replaced.

polcak added a commit that referenced this pull request May 13, 2020
@polcak
Copy link
Owner

polcak commented May 13, 2020

70199a9 merged by 45c9f84 instead of e0c1142

polcak pushed a commit that referenced this pull request May 18, 2020
* Freeze the wrapped object, otherwise, page script can simply 'delete' the wrapper to get original functionality
* Introduce a proxy object with the possibility to detect it by is_proxy Symbol

Note that both additions increases the fingerprintability of the
browser, see:
Michael Schwarz, Florian Lackner, and Daniel Gruss. 2019. JavaScript Template
Attacks: Automatically Inferring Host Information for Targeted Exploits. In
Network and Distributed Systems Security (NDSS) Symposium.

See also #39 for more details about the context of this work.
@polcak
Copy link
Owner

polcak commented May 18, 2020

Looking at b9e72fa, I am not completely sure why do we modify the wrap_code function. I prepared b77942c. Can you specify why do we modify the Proxy?

@polcak polcak added this to the 0.3 milestone May 18, 2020
@PeetHornak
Copy link
Author

I added is_proxy symbol, to not wrap objects that were already wrapped once, this ensures that mapping on memory is not done again. More on that can be found in the paper.

wrap_code doesn't have much impact. It just ensures that undefined statements are not in injected code.
I also added ${wrapper.post_replacement_code || ''} inside define_page_context_function which I used for executing code after replacement but still in the same scope. I noticed that this is missing in ff_bug version of this function.

@polcak
Copy link
Owner

polcak commented May 18, 2020

In the JS Zero paper?

@PeetHornak
Copy link
Author

PeetHornak commented May 18, 2020

I'm not sure to which part was this question aimed at.

is_proxy was not proposed in JS Zero paper, but as I mentioned, there was a lot of functionality missing. Yet I'm aware of bigger fingerprintability caused by this. I wasn't sure how to do it otherwie.

ff_bug version meaning function define_page_context_function_ffbug that is replacing define_page_context_function in case of blocking by CSP on Firefox. It should be added there to ensure same behavior on both browsers.

EDIT: To be honest I'm not sure where to add this piece of code in case of define_page_context_function_ffbug since I didn't encounter issues with CSP myself and don't know the behavior.

@polcak
Copy link
Owner

polcak commented May 18, 2020

It was about "I added is_proxy symbol, to not wrap objects that were already wrapped once, this ensures that mapping on memory is not done again. More on that can be found in the paper." I am not sure which paper. Do you mean your thesis draft?

For more information about the FF bug see #25. As mentioned in the comment, the fix is not working correctly.

@PeetHornak
Copy link
Author

Oh yes. I meant the thesis draft I sent you.

For more information about the FF bug see #25. As mentioned in the comment, the fix is not working correctly.

Thank you, I will have a look at it.

…it will always return as it will be stored in lastValue. (Google maps issue)
@polcak
Copy link
Owner

polcak commented May 19, 2020

Going back to the Proxy matter: b9e72fa wraps the Proxy even if every wrapping is disabled (level 0). Why is that necessary?

Anyway, I split b9e72fa into 34cd8d3 and d4bb269 but some code from the original is still missing.

Please can you polish this whole threads of commits. It is very time consuming to go the commits one-by-one and fix the same issues again and again. Often, the commits cover more than one atomic change and the commit log does not explain why is the code needed in the presented form.

I suggest reading some of the articles about writing commits, such as https://duckduckgo.com/html?q=how%20to%20write%20a%20good%20commit, then, rewriting the history and then pulling again.

@polcak
Copy link
Owner

polcak commented May 19, 2020

Also, bd60552 fixes an issue introduced by ae15072. Please fix ae15072 which is a part of this commit series.

@polcak
Copy link
Owner

polcak commented May 19, 2020

#41 fixes the issue of inserting code when there is nothing to be wrapped. However, I am still concerned that blindly wrapping Proxy is not a good approach. There should be a valid reason to do so.

@polcak polcak linked an issue May 19, 2020 that may be closed by this pull request
@polcak polcak marked this pull request as draft May 19, 2020 15:24
@polcak polcak added the enhancement New feature or request label May 19, 2020
@PeetHornak
Copy link
Author

PeetHornak commented May 19, 2020

Yes I completely agree with you about the Proxy. I will add it as a part of array wrapping, because it is used only there.

I will split these changes into separate pull requests, so we can have separate discussion a not everything here. As you said, it will be easier for you to review and then we can merge each pull request as one security measure.

@PeetHornak
Copy link
Author

I created pull request for each of the sub tasks so it is easier to review and comment.
Thus we can probably close this pull request.
Hope it is better now, sorry for making a mess out of commits.

@polcak polcak closed this May 22, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Port functionality from ChromeZero
2 participants