Best way to read attributes in nsScriptLoader?

[jeremy...@gmx.ch]

Hi together,

I'm experimenting with the script-nonce idea to push CSP forward. While acquainting myself with the code and testing different approaches, I stumbled upon the question, if I could use the well known class attribute inside a <script> tag.

Here is what I'm trying to do:
In nsScriptLoader::ProcessRequest I wish to gain access to the values of the class attribute of the current script tag and analyse them. What would be the best/easiest way to do this?


Here is what I've thought on this:
- nsScriptLoader::ProcessRequest is called with the parameter aRequest, for which several methods, like GetScriptType are called inside the function. I understand, that aRequest stands for the <script> tag that is currently processed.

- aRequest is a pointer of the type nsScriptLoadRequest. So I looked up nsScriptLoadRequest, but none of the methods defined there enables me to read the class attribute.

- So I guessed I'd have to use a more general class and looked further for functions with promising names and found getHTMLAttr() in nsGenericHTMLElement.h, ParseAttribute() in HTMLScriptElement and GetClassAttributeName() in nsStyleElement. I'm completely unsure, if any of these could help me or if they are reasonable for my target at all.

Best regards,
Jeremy

[Boris Zbarsky]

On 3/27/13 8:00 AM, jeremy...@gmx.ch wrote:
> In nsScriptLoader::ProcessRequest I wish to gain access to the values of the class attribute of the current script tag and analyse them. What would be the best/easiest way to do this?

Change the type of "scriptElem" to nsCOMPtr<Element>, and then use
either scriptElem->GetAttr() or scriptElem->GetClassList() depending on
whether you want the string or the tokenized version.

-Boris

[jeremy...@gmx.ch]

Thank you Boris. With you mentioned changes, suppose I only want to check, if the script element owns a class value "xyz", i.e. <script class="xyz">

Could I use something like this?

int32_t containsClassVar = scriptElem->FindAttrValueIn(aNameSpaceID,
"class", "xyz", eCaseMatters);

// proceed depending on value of containsClassVar

What do I have to use for "aNameSpaceID"? This is not clear to me.

Jeremy

[Boris Zbarsky]

On 3/27/13 11:34 AM, jeremy...@gmx.ch wrote:
> Thank you Boris. With you mentioned changes, suppose I only want to check, if the script element owns a class value "xyz", i.e. <script class="xyz">
>
> Could I use something like this?
>
> int32_t containsClassVar = scriptElem->FindAttrValueIn(aNameSpaceID,
> "class", "xyz", eCaseMatters);

How do you want <script class="xyz abc"> to behave? And also,
nsGkAtoms::_class instead of "class", and the next argument should be an
atom pointer array... But I suspect all that is moot given what you're
likely to say about the "xyz abc" case. ;)

> What do I have to use for "aNameSpaceID"?

kNameSpaceID_None, if you're doing this.

-Boris

[Devdatta Akhawe]

I thought the script-nonce directive uses `nonce' attribute that
should only contain the nonce and nothing else. That would mean you
only need to do a full string compare and not worry about "xyz abc"
case.

https://dvcs.w3.org/hg/content-security-policy/raw-file/tip/csp-specification.dev.html#script-nonce

-dev

On 27 March 2013 09:21, Boris Zbarsky <bzba...@mit.edu> wrote:
> On 3/27/13 11:34 AM, jeremy...@gmx.ch wrote:
>>

>> Thank you Boris. With you mentioned changes, suppose I only want to check,
>> if the script element owns a class value "xyz", i.e. <script class="xyz">
>>
>> Could I use something like this?
>>
>> int32_t containsClassVar = scriptElem->FindAttrValueIn(aNameSpaceID,
>> "class", "xyz", eCaseMatters);
>
>

> How do you want <script class="xyz abc"> to behave? And also,
> nsGkAtoms::_class instead of "class", and the next argument should be an
> atom pointer array... But I suspect all that is moot given what you're
> likely to say about the "xyz abc" case. ;)
>
>

>> What do I have to use for "aNameSpaceID"?
>
>

> kNameSpaceID_None, if you're doing this.
>
> -Boris
>

> _______________________________________________
> dev-security mailing list
> dev-se...@lists.mozilla.org
> https://lists.mozilla.org/listinfo/dev-security

[Boris Zbarsky]

On 3/27/13 12:27 PM, Devdatta Akhawe wrote:
> I thought the script-nonce directive uses `nonce' attribute that
> should only contain the nonce and nothing else.

That could well be, but Jeremy was explicitly asking about the 'class'
attribute...

If you just want to check that the value of an attribute is something in
particular, use AttrValueIS.

-Boris

[jeremy...@gmx.ch]

@Dev: You're right about the script-nonce directive. However, at the moment I'm thinking in multiple directions and not everything I ask might sound logical on it's on. I hope things will become more clearly soon. In this specific case I'm actually looking for a way to check, if a script element contains some specific class value.

@Boris: for <script class="A B C"> I only want to know if script is member of class "A", a mere true/false descision. If the class attribute contains also a value "B", "C" or other values is completely uninteresting and can (and should) be ignored for now. If it does contain "A" the script will be processed, if it does not, it will be skipped. Referring to your last post, would

int32_t containsClassVar = scriptElem->FindAttrValueIn(kNameSpaceID_None,
nsGkAtoms::_class, NS_LITERAL_STRING("xyz"), eCaseMatters);

work correctly or should I better use

bool containsClassVar = scriptElem->AttrValueIs(kNameSpaceID_None,
nsGkAtoms::_class, NS_LITERAL_STRING("xyz"), eCaseMatters);

???

Jeremy

[Boris Zbarsky]

On 3/27/13 2:35 PM, jeremy...@gmx.ch wrote:
> @Boris: for <script class="A B C"> I only want to know if script is member of class "A", a mere true/false descision.

OK, so scriptElem->GetClassList()->Contains() should do that for you.

> int32_t containsClassVar = scriptElem->FindAttrValueIn(kNameSpaceID_None,
> nsGkAtoms::_class, NS_LITERAL_STRING("xyz"), eCaseMatters);
>
> work correctly

That won't even compile.

> bool containsClassVar = scriptElem->AttrValueIs(kNameSpaceID_None,
> nsGkAtoms::_class, NS_LITERAL_STRING("xyz"), eCaseMatters);

That will give you the wrong behavior (class="xyz abc" will return false).

-Boris

[jeremy...@gmx.ch]

OK, I've reviewed your input and built a minimal example that compiles without errors, but doesn't do what it should. In nsScriptLoader::ProcessScriptElement I added after line 470:


request->mElement = aElement;
nsCOMPtr<Element> currentScriptElem = do_QueryInterface(request->mElement);

nsAutoString classValue(NS_LITERAL_STRING("aaa"));

ErrorResult myError;

if (!currentScriptElem->GetClassList()->Contains(classValue, myError))
return false;


I've tested this with a simple HTML file

<html><head><body>
<script id="script1" class="aaa"> // some code here </script>
<script id="script2" class="bbb"> // some code here </script>
<script id="script3"></script>
</body></html>

The changes are supposed to execute only the code of "script1", as it has the correct class value "aaa". Scripts 2 and 3 don't have the correct value or have no class value at all, so the Contains() method should return "false" and the condition should be triggered and the execution of the current script should be skipped. But it doesn't - all scripts are executed.

Is there some error in my code or did I misunderstand the functioning of ProcessScriptElement()? How can I make sure to abort the processing of a script that FF is about to execute, after I have examined it?

Regards,
Jeremy

[Boris Zbarsky]

On 4/5/13 10:35 AM, jeremy...@gmx.ch wrote:
> OK, I've reviewed your input and built a minimal example that compiles without errors, but doesn't do what it should. In nsScriptLoader::ProcessScriptElement I added after line 470:

Which line is that, exactly, in your version of nsScriptLoader? My line
470 doesn't even have a "request" object, so I'm not sure which code
exactly you're modifying.

-Boris

[jeremy...@gmx.ch]

http://mxr.mozilla.org/mozilla-central/source/content/base/src/nsScriptLoader.cpp#470

The code was added between

468 // Step 14. in the HTML5 spec
469 nsresult rv = NS_OK;
470 nsRefPtr<nsScriptLoadRequest> request;

and

471 if (aElement->GetScriptExternal()) {
472 // external script
473 nsCOMPtr<nsIURI> scriptURI = aElement->GetScriptURI();

Jeremy

[Boris Zbarsky]

On 4/5/13 12:04 PM, jeremy...@gmx.ch wrote:
> The code was added between

> 470 nsRefPtr<nsScriptLoadRequest> request;
>
> and
>
> 471 if (aElement->GetScriptExternal()) {

OK, and you actually recompiled everything? Because your code as
written dereferences request, so it would crash if it actually ran....

-Boris

[jeremy...@gmx.ch]

It could very well be, that I missed something when recompiling. I ran

$ python ./../../../build/pymake/make.py

twice, first from

/obj.../content/base/

and second from

/obj.../layout/build

because I've read somewhere, that the second one is always necessary when making changes to /content

Jeremy

[jeremy...@gmx.ch]

Oh, and I also changed

request->mElement = aElement;
nsCOMPtr<Element> currentScriptElem = do_QueryInterface(request->mElement);

to

nsRefPtr<nsScriptLoadRequest> myRequest;
myRequest->mElement = aElement;
nsCOMPtr<Element> currentScriptElem = do_QueryInterface(myRequest->mElement);

Jeremy

[Boris Zbarsky]

On 4/5/13 12:26 PM, jeremy...@gmx.ch wrote:
> It could very well be, that I missed something when recompiling. I ran
>
> $ python ./../../../build/pymake/make.py
>
> twice

May be worth just doing a toplevel build.

Or at least adding a printf to that function to see if your changes are
in fact being picked up.

That function most definitely gets called for every <script> in HTML, so
if a printf doesn't get hit the problem is the recompile.

-Boris

[Ian Melven]

you also need to build toolkit/library to rebuild xul.dll with your binary code changes
(you're using pymake so you must be on Windows :) )

thanks,
ian

[jeremy...@gmx.ch]

I did what Ian said and built also toolkit/library. It runs through, but when I try to start up Nightly, it breaks with an NSGlue_Assertion error:

ASSERTION: You can't dereference a NULL nsRefPtr with operator ->().:'mRawPtr != 0', file c:\mozilla-source\mozilla-central\obj-i686-pc-mingw32\dist\include\nsAutoPtr.h, line 1028

I tried ignore and retry, but it doesn't help.

Jeremy

[Boris Zbarsky]

Well, right. See "Because your code as written dereferences request, so

it would crash if it actually ran".

Don't do that. Just QI the argument directly.

-Boris

[jeremy...@gmx.ch]

Boris, you're right. But somehow I still don't get it. I've changed

nsRefPtr<nsScriptLoadRequest> myRequest;
myRequest->mElement = aElement;
nsCOMPtr<Element> currentScriptElem = do_QueryInterface(myRequest->mElement);

to

nsCOMPtr<Element> currentScriptElem = do_QueryInterface(aElement);

and still get the same error. Isn't this, what you mean by "QI the argument directly"? I mean, how could this dereference request?

Jeremy

[jeremy...@gmx.ch]

It finally works now. I had to put my code BEFORE

// Step 14. in the HTML5 spec

nsresult rv = NS_OK;
nsRefPtr<nsScriptLoadRequest> request;

and everything was fine. However, I'd still like to understand why "request" gets dereferenced when my code stands below these lines. I just don't see it.

And thanks for all the patient comments on my beginner's questions :-)
Jeremy

[Josh Matthews]

In at least one snippet you pasted, you were doing
>nsRefPtr<nsScriptLoadRequest> request;
>request->mElement = aElement;

nsRefPtr points to null by default.

Cheers,
Josh

[jeremy...@gmx.ch]

Thank you Josh!

Jeremy