Let the browser choose the SQL

Vechni

While doing a code review for possible XSS vulnerabilities I found something worse:

And yeah, the action class will read these variables to build an SQL string.

derula

@Vechni said:

And yeah, the action class will read these variables to build an SQL string.

Wait, let me highlight this.

<input type="hidden" name="orderBy" value="<bean:write name="orderBy"/>">

(tags, attributes, values, bullshit)

This is how I would parse it if I was a browser.

Let's hope real browsers are more fault-tolerant.

TarquinWJ

[Looks like I proved my point by upsetting the server's markup parser ;) ]

TarquinWJ

@derula said:

Let's hope real browsers are more fault-tolerant.

They're not. Presumably this is community server treating both unencoded < and encoded < as < (and likewise for " and ").

derula

@TarquinWJ said:

They're not. Presumably this is community server treating both unencoded < and encoded < as < (and likewise for " and ").

Ah, yes. I tend to forget there are actually people using the WYSIWYG editor and not previewing. Nevermind then. It's still a WTF, even properly encoded.

jpa

@derula said:

@Vechni said:
And yeah, the action class will read these variables to build an SQL string.

Wait, let me highlight this.

<input type="hidden" name="orderBy" value="<bean:write name="orderBy"/>">

(tags, attributes, values, bullshit)

This is how I would parse it if I was a browser.

Let's hope real browsers are more fault-tolerant.

I would assume that the <bean: -part is replaced server-side and never gets to the browser. Though I'm not sure if such construct is allowed for the server-side parsing either, as I do not know the language in question.

Vechni

bean:write is a struts tag that is pretty much equivalent to page.write(). view source will show you a page with stuff like value="order by auto_part_id desc".

psiphiorg

@derula said:

Wait, let me highlight this.

<input type="hidden" name="orderBy" value="<bean:write name="orderBy"/>">

(tags, attributes, values, server-side replacement: the browser never sees this)

This is how I would parse it if I were a browser.

Let's hope real browsers are more fault-tolerant.

FTFY.

davidh

Bulb

@psiphiorg said:

@derula said:
Wait, let me highlight this.

<input type="hidden" name="orderBy" value="<bean:write name="orderBy"/>">

(tags, attributes, values, server-side replacement: the browser never sees this)

This is how I would parse it if I were a browser.

Let's hope real browsers are more fault-tolerant.
FTFY.

davidh

Actually, that would be how the programmer intended it. However, the quoted string ends with the first quote, so the browser should parse it as:

<input type="hidden" name="orderBy" value="<bean:write name="orderBy"/>">

And actually print the "> as a plain text. Obviously the server would have hard time understanding what it got back on the form submission.

Weng

are you honestly THAT FUCKING DENSE?

<bean:write name="orderBy"/>

will be replaced, BEFORE THE HTML IS EVER SENT FROM THE SERVER TO THE CLIENT, with something like 'ORDER BY penisSize'

It is NEVER transmitted to the browser and is thus NEVER returned as a form submission item. If you have ever developed for the web in any language, you have NO EXCUSE for not understanding this concept.

dhromed

I'm continually amazed at the cleverness of all browsers to parse the <%%>-tag, even though it's commonly used as malformed markup. It's like magic, you know?

Wait.

dhromed

@Weng said:

are you honestly THAT FUCKING DENSE?

You wrote what I thought, but I figured I'd be all sarcastically suave.

Meep3d

@Weng said:

are you honestly THAT FUCKING DENSE?
<bean:write name="orderBy"/>
will be replaced, BEFORE THE HTML IS EVER SENT FROM THE SERVER TO THE CLIENT, with something like 'ORDER BY penisSize'
It is NEVER transmitted to the browser and is thus NEVER returned as a form submission item. If you have ever developed for the web in any language, you have NO EXCUSE for not understanding this concept.

The contents of the hidden input field is returned though and there can be no other reason for this than to build the query when it gets passed back, due to the inherent sessionless nature of html.

Especially concerning is the queryType field. I would have no doubt that, even escaped, someone could do damage with this if they wanted.

derula

You know you're a good troll when you troll people unintentionally.

@dhromed said:

@Weng said:
are you honestly THAT FUCKING DENSE?
You wrote what I thought, but I figured I'd be all sarcastically suave.

It started that way, but from the third word onward it kind of turned bad...

dhromed

@Zecc said:

It started that way, but from the third word onward it kind of turned bad...

I suppose it did.

Sorry for my tone, offendees, and sorry for Weng's tone, but that doesn't mean Weng is wrong. He's right, and I have no idea where these comments about supposedly malformed html are coming from.

Just for the sake of clarification: I was referrering to Weng's post and not yours.

morbiuswilters

This thread is making me sad, for so very many reasons.

morbiuswilters

@Vechni said:

bean:write is a struts tag that is pretty much equivalent to page.write(). view source will show you a page with stuff like value="order by auto_part_id desc".

TRWTF is that it is using a quasi-XML scheme for the struts tags but that any XML parser would choke on the rest of the HTML. So it uses an HTML parser to pull apart the page and look for any attributes that match the Struts schema, then an XML parser to parse the tag and do the replacement? That seems so gross. At least scripting languages that allow embedding in HTML actually give you access to the full language.

bstorer

@morbiuswilters said:

TRWTF is that it is using a quasi-XML scheme for the struts tags but that any XML parser would choke on the rest of the HTML. So it uses an HTML parser to pull apart the page and look for any attributes that match the Struts schema, then an XML parser to parse the tag and do the replacement? That seems so gross.

Welcome to Struts. Get used to it.

Weng

@derula said:

You know you're a good troll when you troll people unintentionally.

I hope it's malicious rather than unintentional. If it's unintentional may god help us all.

morbiuswilters

@Weng said:

@derula said:
You know you're a good troll when you troll people unintentionally.
I hope it's malicious rather than unintentional. If it's unintentional may god help us all.

Alcohol is my primary influence. I don't know if that counts as intentional or not, though.

dhromed

@Zecc said:

Just for the sake of clarification: I was referrering to Weng's post and not yours.

What can I say.

I'm a motherfucking bleeding heart, bitch. ;)

toth

But...they're hidden! So no evil hacker will be able to see that they're there! It's 100% secure!