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

NoScript domrect jitters #166

Closed
Thorin-Oakenpants opened this issue May 20, 2022 · 42 comments
Closed

NoScript domrect jitters #166

Thorin-Oakenpants opened this issue May 20, 2022 · 42 comments

Comments

@Thorin-Oakenpants
Copy link
Contributor

https://gitlab.torproject.org/tpo/applications/tor-browser/-/issues/40919

This reminds me, have you noticed NoScript affects dom rect results for a few months now. It's somewhat jittery and then stable after a few reloads. I'm not sure, but it seems to be a result of appending elements, but only the dom rect is affected.

Originally posted by @abrahamjuliot in #34 (comment)

@Thorin-Oakenpants
Copy link
Contributor Author

Thorin-Oakenpants commented May 20, 2022

you had me at jittery ...

I don't have NoScript anywhere, except Tor Browser, which I haven't really been doing any testing in for like six months. Do you notice this from your own observations (i.e you see domrect getting trashed on creepy) or is from results in the backend? Tell me more

@Thorin-Oakenpants
Copy link
Contributor Author

Oh, and was this seen in Tor Browser

@abrahamjuliot
Copy link
Collaborator

abrahamjuliot commented May 20, 2022

So far, only noticed it in Tor Browser starting in Feb or March (on both creepy and TZP). I haven't done a thorough analysis yet, but the same pattern is in JShelter on FF (they imported NoScript code). I observed it on the frontend and accepted it on backend (the results valid).

@Thorin-Oakenpants
Copy link
Contributor Author

OK, I'll make a note of that in the ticket

@Thorin-Oakenpants
Copy link
Contributor Author

I observed it on the frontend and accepted it on backend (the results valid).

Then how did you observe it. Do you mean both TZP and creepy's lie PoCs fail?

@Thorin-Oakenpants
Copy link
Contributor Author

also is it slider related - do you need to be on safer/standard

@Thorin-Oakenpants
Copy link
Contributor Author

I just tried in uptodate alpha and release (both ESR91 based) and it seems stable: I just loaded TZP and checked my line-height and dom rect vales, then did a reload .. etc etc etc - stable AF so far: identical in both. Will keep testing every now and then

@abrahamjuliot
Copy link
Collaborator

safer/standard

It occurs in both. I noticed today that it also affects css pixels. Text metrics and svg rect unaffected.

PoCs fail?

It's on both TZP and creepy, and passes math checks, but randomly fails known zoom level checks. This seems to suggest element styles are altered in some fashion. I'm seeing it on 3 windows machines.

@Thorin-Oakenpants
Copy link
Contributor Author

So I was thinking about this before when I was out walking, and the reason we don't pick up on lies is twofold

  • the known measurement is rooted to the spot in the top left, and it is not affected/"moved" like the domrect elements
  • clientrect is not being spoofed, so the shift method also fails

It would be nice to know WTF is actually changing - I would assume it is the x/y. I wonder if some of the other tests that are affected could be resolved with judicious use of position:fixed or remove padding/margins or set left/top etc? - obviously without wanting to affect any entropy

Need to know more about what causes this, to be able to think about preventing it and/or detecting it

current

<body>
	<!-- domrect -->
	<div id="divrect" class="divrect1">
		<div class="testRect" id="rect6"></div>
		<div class="insideDOMRect">
			<h1 class="testRect" id="rect0">content</h1>
			<span class="testRect" id="rect1">content
		</div>
		<select class="testRect" id="rect3"><option>content</option></select>
		<progress class="testRect" id="rect4"></progress>
		<button class="testRect" id="rect5"></button>
	</div>
.divrect1 { /* SNAP to top left */
  top: 0;
  left: 0;
}

.divrect2 { /* SHIFT test */
  top: 0.25px;
  left: 0.25px;
}


#rect6 { /* KNOWN measure, start top left, FIXED */
	top: 0;
	left: 0;
	width:100px;
	height:100px;
	transform: rotate(45deg);
	padding: 0px;
	position: fixed;
}

@Thorin-Oakenpants
Copy link
Contributor Author

I wonder if NS is doing something like this

javascript:( function(){ let i, elements = document.querySelectorAll('body *'); for (i = 0; i < elements.length; i++) { if(getComputedStyle(elements[i]).position === 'fixed' || getComputedStyle(elements[i]).position === 'sticky') { elements[i].parentNode.removeChild(elements[i]); } } document.body.style.overflow = "auto"; document.body.style.position = "static"; })()

@abrahamjuliot BTW, can you tell me how you know the dom rect result (or other items) are not correct on TZP

@abrahamjuliot
Copy link
Collaborator

As far as I know, NS is always correct math wise, but the hash changes at random. NS is only incorrect on creepy in failing my rigid known zoom tests. I'm currently retesting on TZP, and the jitters no longer occur (it's still an issue on creepy).

@Thorin-Oakenpants
Copy link
Contributor Author

my rigid known zoom tests

this is getting confusing. sticking to one test, are we talking about domrect. Are we talking about the known measurements PoC (which is zoom resistant on gecko)? If so, why the zoom tests - did you mean on chromium?

but more importantly .. how do you know it's failing? Which part is failing - the known PoC or the domrect test itself?

yours sincerely

  • confused as F

@abrahamjuliot
Copy link
Collaborator

abrahamjuliot commented Jun 5, 2022

Yes, domrect. NS fails the known PoC (testing in gecko only) due to unknown results, but only on the creep main page. It's passing the domrect test page and TZP with no jitters. I think you are correct that layout positioning is altered by the extension in some fashion.

@Thorin-Oakenpants
Copy link
Contributor Author

OK, still confused. So the known PoC fails so you record the domrect test as trash. Question: are the domrect results also altered? So the way I see it, is it works exactly as intended. Except ... NS is not actually trying to mess directly with clientrect, so you/we need to harden it from being moved around

in other words, you noticed it fail because TZP wrapped domrect in red lies colors (or for you trashed with a less sexy red than TZP's), but didn't expect it to fail. gotcha

from your comment 16 days ago

So far, only noticed it in Tor Browser starting in Feb or March (on both creepy and TZP).

But I haven't changed anything. So IDK ... TB updated with a new NS version (I think). If TZP failed in the past it can fail again.

TZP uses set elements. For the known poc failing, I think we might just need to reapply the css rule(s) right before each test. div element style = blah ??

<body>
	<!-- domrect -->
	<div id="divrect" class="divrect1">
		<div class="testRect" id="rect6"></div> // <- known poc (diamond)
#rect6 {
	top: 0;
	left: 0;
	width:100px;
	height:100px;
	transform: rotate(45deg);
	padding: 0px;
	position: fixed !important;
}

@Thorin-Oakenpants
Copy link
Contributor Author

TZP uses set elements

Do you create the domrect elements on the fly? That might explain it a bit if NS has a listener?

@Thorin-Oakenpants
Copy link
Contributor Author

BTW, that bookmarket I posted is something people use (or similar in extensions I guess) to remove advs and shit that follow you down the page etc - this https://news.ycombinator.com/item?id=31625804 made me think of this NS jitter issue

so we should harden regardless - a bit like how we reset the span style in fonts

@Thorin-Oakenpants
Copy link
Contributor Author

PPS: you could test your creepy jitter by debugging the known PoC style at the end

@abrahamjuliot
Copy link
Collaborator

abrahamjuliot commented Jun 6, 2022

are the domrect results also altered?

It alters all domrect coordinates. CSS pixels are altered too (top-right and bottom left here), but not SVG elements.

domrect elements on the fly?

I think that must be the issue. Debugging is difficult since it occurs only once every other minute after repeat refreshing. 😭

One thing I'm seeing is some results reoccur and can still pass the known value test. For example, here, both results pass and were previously seen a number of days ago (the 2nd is the expected result).

bookmarket

Nice. I need to try that. I use uBO sometimes to manipulate pages. Created this to ban annoying styles in emails. 😁

@Thorin-Oakenpants
Copy link
Contributor Author

Created this to ban annoying styles in emails

I just force plain text in all emails (and block external requests)

@Thorin-Oakenpants
Copy link
Contributor Author

bookmarket

I often buy books there .. clearly I need more time off

@Thorin-Oakenpants
Copy link
Contributor Author

Debugging is difficult since it occurs only once every other minute after repeat refreshing

I don't follow, just debug every time .. when it occurs check your console.

So I'm not quite following this. The known Poc is set top left 0 fixed, and (for TZP at least) along with the other domrect items is inside the first div on the html page. So even if I nudge the parent div down the page, lets say I add a line break <br>, the known PoC is still stable

here I have added extra lines before and inside that div. This is the parent div we use for shifting
fixed

So the known PoC will always return true (as long as something doesn't change it's style. So that passes, capisce? But the domrect elements after the known PoC element are not fixed, so they can move, and alter the FP.

But my domrect FP is fe42e2d866f345af05d33d7f4651e845937a58d4 in the above picture and normally. But that's because I also force the parent div to fixed - do you?

Still, I wonder what made TZP fail in the past

@Thorin-Oakenpants
Copy link
Contributor Author

In order to use clientrect with confidence in any section, I promise it as a prereq; Note: I will be doing away with the current domrect test which is a bit useless, and in future I will have a much bigger range of elements (especially widgets) - single pass, single method

const get_isClientRect = () => new Promise(resolve => {

isClientRect = aClientRect.indexOf(true)

So isClientRect determines what I use and if it is untrustworthy - currently gecko only

Untitled-1

I updated https://arkenfox.github.io/TZP/tests/domrectspoof.html and was using this to see if there was some sort of blink standard, but have found weird results (and I don't mean how Element,getBoundingClientRect can differ from the others, and I don't mean changes in zoom. Admittedly, these weird results only seems to happen at very large zooms and only after scrolling

I'll post more in a minute. Note I want to move away from the shift method if possible. With FF, I can already determine if the amount altered is the same per measurement (chameleon), or random (canvasblocker), but it may be that we need the shift method for blink

@Thorin-Oakenpants
Copy link
Contributor Author

Thorin-Oakenpants commented Sep 8, 2022

here are some blink notes: using https://arkenfox.github.io/TZP/tests/domrectspoof.html

index order = obj.x, obj.left, obj.y, obj.top, obj.width, obj.height, obj.right, obj.bottom

			// chrome: math PoCs?
				// x === left
				// y === top
				// width == height
				// right == bottom
			/* examples
what's the correlation/proof here: aspect ratio, area
width+height/2 vs right+bottom/2

Element.getBoundingClientRect can be different to the others
scroll around a bit and hit F5 and you get inconsistencies: so far only on zoom 500

zoom 500
0+1: -20.710678100585938
2+3: -20.710678100585938 <-- 0-3 the same
4+5: 141.42137145996094
6+7: 120.710693359375

0+1: -20.710668563842773
2+3: -20.710668563842773 <- 0-3 the same
4+5: 141.42135620117188
6+7: 120.7106876373291

0+1: -20.710668563842773
2+3: -20.710668563842773 <- 0-3 the same
4: 141.42137145996094
5: 141.42135620117188 <- 4 + 5 different
6: 120.71068954467773
7: 120.7106876373291 <- 6+7 different

zoom 400
0+1: -20.710678100585938
2+3: -20.710678100585938 <- 0-3 the same
4+5: 141.42135620117188
6+7: 120.71067810058594

zoom 300
0+1: -20.710674285888672
2+3: -20.710683822631836 <- 0-3 not the same
4+5: 141.42135620117188
6: 120.7106819152832
7: 120.71067237854004 <-- 6+7 different

@Thorin-Oakenpants
Copy link
Contributor Author

so I thought some quick lie tests would be x === left, y === top, and even the first four must match, but that's not always true: x and y can differ, there's even cases where width + height + right + bottom all differ - and besides, if the extension is adding the same amount of noise, the test would pass

what exactly are you using in creepy? @abrahamjuliot if I remember rightly it;s width - left = top - height or whatever ?

@abrahamjuliot
Copy link
Collaborator

abrahamjuliot commented Sep 9, 2022

shift method for blink

The shift method works on Bromite and Ungoogled Chromium, but I think only because they apply a fresh scale factor on every DOM reflow. There are some customized blink engine software that manage to bypass it.

I use

lied = (
  right - left !== width ||
  bottom - top !== height ||
  right - x !== width ||
  bottom - y !== height
)

I noticed if I apply a custom matrix transform and run it in the console on some random heavily styled site, the dimension equality is not the same and needs to be refactored. Page styles or injected styles can sometimes leak into the elements and affect positioning. This can be prevented if we use an iframe for measuring elements or reset the styles. But, random styles injected from extensions can still impact the iframe.

@Thorin-Oakenpants
Copy link
Contributor Author

OK, so that's the same pair of tests and covering x == left and y == top, which always seems to be the case (I tidied up my examples - 0+1 are always the same, 2+3 are always the same)

same result

lied = (
  x !== left ||
  y !== top ||
  right - x !== width ||
  bottom - y !== height
)

@Thorin-Oakenpants
Copy link
Contributor Author

9a55fa8

So, you can load the test in chrome, zoom up to 500, resize the window, as you get smaller enough, move the scrollbars and continue resizing, etc, then zoom down one, repeat, then zoom down one, repeat

Then in the console you can type

  • outputBlink() and it will list any new hashes we don't already know about
  • testBlink() and it tests against your lies poc
    • only 1 so far produces a false positive: 500 zoom and I moved the document page via scrollbars off default position

That's 35 known results on windows with preset zoom levels and dpi = 1, devicepixelratio = 1, systemscaling = 1

I suspect that there are many more combos

@Thorin-Oakenpants
Copy link
Contributor Author

the one false positive

testBlink()
domrectspoof.html:251 2d2f0e42

  • right - x !== width, 120.71068954467773, -20.710668563842773, expected 141.42137145996094, got 141.4213581085205, diff -0.0000133514404296875

So the shift method was the first method I came up with, but if an extension like chameleon shifts everything by the same amount, then this test fails, which is why I came up with the known measurement PoC

So I have to ask, does this method (4 x lies check) detect those that shift everything by the same amount. and the answer is I am too tired to think about it right now - i.e the shift method is not required IMO, ever (off the top of my head)

@abrahamjuliot
Copy link
Collaborator

I got 2d2f0e42 too.

does this method (4 x lies check) detect those that shift everything by the same amount

No, but if we use a matrix to Warp and Skew the rectangle, and then adjust the expected calculations accordingly, shifting by the same amount can still fail. I'm not fully understanding why it fails.

/* style */
#rect1.matrix {
  transform: matrix(1.11, 2.0001, -1.0001, 1.009, 150, 94.4);
}
lied = !(
 (matrixRight - matrixLeft) == matrixWidth && (matrixRight - matrixX) == matrixWidth &&
 (matrixBottom - matrixTop) == matrixHeight && (matrixBottom - matrixY) == matrixHeight
)

I like the known measurements PoC the best.

@Thorin-Oakenpants
Copy link
Contributor Author

Thorin-Oakenpants commented Sep 13, 2022

not sure what you mean - are you saying that our false positive passes the matrix test, or that it still fails?

So two options

  • accept that weird results can happen, such as unlikely zoom 500 + reposition page via scrollbars
    • in reality, if these results are NOT STABLE then they should be detected as untrustworthy: e.g. if something like another extension is affecting the result then we're doing our job? am i missing something here?
    • not to be confused with enforcing and fixing this so we get a stable result
  • possibly, so far: only one weird result to go on
    • count the lies: our false positive only affected 1 of 4

refresher

lied = (
  x !== left ||
  y !== top ||
  right - x !== width ||
  bottom - y !== height
)

now that depends entirely on the extension. The first two lies will never trigger if legit, and I would suspect an extension would never trigger these either (if they did then that's even better). I honestly can't see any decent extension not triggering at least two lies (but it is possible)

IDK how prevalent this "jitter" (ignoring NoScript) is, but perhaps you could start collecting a lieCount, or even better a lieMatrix so we know which ones are being affected

@Thorin-Oakenpants
Copy link
Contributor Author

We could also collect something like pageOffsetX,Y ? - e.g. if the page is offset and liecount is only 1 and it's item 3 then false positive - BUT it;s still untrustworthy ? IDK

@abrahamjuliot
Copy link
Collaborator

abrahamjuliot commented Sep 14, 2022

accept that weird results can happen

I like this idea. I am less inclined to rely on it, but it can be used to detect lies by comparing to CSS pixels. For example, if domrect is unstable by zoom, css pixels should be too, and everywhere a known css pixel fingerprint appears, it should have a known domrect paired with it.

matrix test

Zoom settings and CSS transform scale should pass the matrix test since the engine calculates the dimensions, but if an offset is added to each dimension, this can fail the matrix test.

@Thorin-Oakenpants
Copy link
Contributor Author

For example, if domrect is unstable by zoom

It depends what is unstable. If it's x/y then we don't really use those in measurements - it's mostly width/height. I'm actually collecting the full diffs for FF and can categorize two anti-FP fingerprints - scope (what measurements: x, y, etc) and noise ( same chameleon style or different CB style - because we know the correct measurement - can't always do that in blink AFAICT)

Zoom settings and CSS transform scale should pass the matrix test since the engine calculates the dimensions, but if an offset is added to each dimension, this can fail the matrix test.

grr. imma explain again :) so we have a false positive (with a single lie out of the four), does it pass the matrix test?

I seriously need a break from this. Matrix test is not the same as shift test (which I want to ignore since it would be bypassed by eg chameleon style spoofing), right?

don't answer, I will just do some tests. There's a key here to eliminate those false positives, but I guess the question is do we want to, they're not stable (or are they? - so many factors not tested: dpi, device pixel ratio etc)

@Thorin-Oakenpants
Copy link
Contributor Author

https://arkenfox.github.io/TZP/tests/domrectspoofratio.html - not needed for FF of course - zoom in and out etc to auto rerun (a history is kept)

So this focuses only on width/height: but relies on known results

let knownBlink = [
	// standard
	0.5, 0.4999999427781113, 0.5000000572218888, 
	// when page is scrolled so no longer top left = 0
	0.4999999427781178,
	0.4999998855562356,
]

Given all it takes is either width or height to change by .000000000000001 or whatever, and there are too many factors (dpi, devicePixelRatio, zooming, the page not being top left (i.e scrolled), and numerous OSes not tested), then I think this wee test I whipped up is a bit naff, except that it lessened the variables. But also, that it only checks width/height - still, if they're fucked with the rest is suspect as well TBH, and width/height are usually the only values we use in metrics (although maybe left is useful in some metrics)

Still, was an enjoyable 15 mins :)

@Thorin-Oakenpants
Copy link
Contributor Author

It works perfectly on my android !! maybe I'm onto something?

@Thorin-Oakenpants
Copy link
Contributor Author

Thorin-Oakenpants commented Jan 3, 2023

@abrahamjuliot is this still an issue for you? PS: when I eventually upload my refactoring, TZP will only run on gecko, plus I've never seen this issue on TZP (unless I'm reproducing it by moving the content page when zoomed in those PoCs, and even then only on blink)

also, NoScript no longer shows any proto/proxy lies - I thought it used to : it also no longer shows tampered iframe doc properties (i.e expected items moved to after constructor). Might have to install an old NS version somewhere to confirm

@abrahamjuliot
Copy link
Collaborator

I'm seeing a minimal number of jitters affecting all element signals (rects, fonts, and svg), but it seems limited to the initial load on browser start up and does not affect TZP.

I did just notice, with NoScript on or off, the glyphs fingerprints on TZP appear random per execution.

NoScript no longer shows any proto/proxy lies

Nice. It looks clean now.

@Thorin-Oakenpants
Copy link
Contributor Author

glyphs fingerprints on TZP appear random per execution

every run I'm adding an extra tofu

TZP/js/fonts.js

Line 1307 in 42b20f1

fntCodeUsed.push('0xFFFF') // ensure one tofu

good catch 👍

@Thorin-Oakenpants
Copy link
Contributor Author

actually, what I'm doing is essentially this

	let fntCodeUsed = fntCode  // fntCode: master global var for lookup array of 42 codes, no '0xFFFF'
	filter_tofu() // skipped if TB or android otherwise, removes tofu codes
	fntCodeUsed.push('0xFFFF') // ensure one tofu

but here's what happens (TB or android)

  • for some reason, and I just don't get it - fntCode is an array not an object, fntCode is getting the tofu added each time. So each run it increases by one tofu. WTF!

wtf


anyway, I'll patch it to only set fntCodeUsed once, and I'll replace that with

fntCode.forEach(function(code) {fntCodeUsed.push(code)})

@Thorin-Oakenpants
Copy link
Contributor Author

but it seems limited to the initial load on browser start up and does not affect TZP

superior TZP coding bro ... what can I say 🤣

is this affecting all engines on creepy or just blink?

@Thorin-Oakenpants
Copy link
Contributor Author

NoScript no longer shows any proto/proxy lies

Nice. It looks clean now.

#4 (comment) - here's what it was - those *Element* items - that saves me loading an old NoScript to double check

@Thorin-Oakenpants
Copy link
Contributor Author

actually, it's quite different now. The items after "Performance" in iframe window properties when the slider is on safer includes 10 more properties after the index position of "Performance" : caused by NoScript (which integrates with the slider) + any code patches by TB themselves

	let aTampered = aProps.slice(aProps.indexOf("Performance")+1)
	let lastItem = aProps[aProps.length - 1]

	let aFilter = ['Event','Location'] // normal FF or TB-standard
	if (isTB && lastItem == "Proxy") { // TB-safer
		aFilter = ["Element","Event","HTMLCanvasElement","HTMLElement","HTMLFrameElement",
			"HTMLIFrameElement","HTMLObjectElement","MediaSource","Proxy","URL","webkitURL",
		]
	}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests

2 participants