1

I am generating links from the following php code. The links appear in the browser, and the generated html code seems fine, however the links are not click-able. I have tested this in IE and FF, and tried to see with FireBug to no avail.

The code to generate my form

$uploadhtml = htmlspecialchars(json_encode("<form action='up.php' method='post'
enctype='multipart/form-data'>
<label for='file'>Filename:</label>
<input type='file' name='file' id='file'/> 
<br />
<input type='hidden' name='pk' value='".$pk."'>
<input type='hidden' name='username' value='".$USERNAME."'>
<input type='submit' name='submit' value='Submit' onclick=\"setTimeout(function() { 
updateByPk('Layer2', '".$pk."', '".$brand."', '".$pg."'); } ),1250);\" />
</form>"), ENT_QUOTES);

The resultant html code:

    <a onclick="makewindows(&#39;&quot;<form action=&#39;up.php&#39; method=&#39;
post&#39;\r\nenctype=&#39;multipart\/form-data&#39;>\r\n<label for=&#39;
`file&#39;>Filename:<\/label>\r\n<input type=&#39;file&#39; name=&#39;file&#39; id=&#39;`file&#39;\/> \r\n<br \/>\r\n<input type=&#39;hidden&#39; name=&#39;pk&#39; value=&#39;
380118179930&#39;>\r\n<input type=&#39;hidden&#39; name=&#39;username&#39; value=&#39;
janmaybach&#39;>\r\n<input type=&#39;submit&#39; name=&#39;submit&#39; value=&#39;
Submit&#39; onclick=\&quot;setTimeout(function() { updateByPk(&#39;Layer2&#39;, 
&#39;380118179930&#39;, &#39;Ed Hardy&#39;, &#39;1&#39;); } ),1250);\&quot; 
\/>\r\n<\/form>&quot;&#39;); return false;" href="#">Upload files</a>

I guess it's a JavaScript error, but I don't know how to pinpoint it?

edit: The html code without ENT_QUOTES:

<a href="#" onclick="makewindows('&quot;<form action='up.php' method='post'\r
\nenctype='multipart\/form-data'>\r\n<label for='file'>Filename:<\/label>\r\n<input 
type='file' name='file' id='file'\/> \r\n<br \/>\r\n<input type='hidden' name='pk' 
value='380118185183'>\r\n<input type='hidden' name='username' value='janmaybach'>\r
\n<input type='submit' name='submit' value='Submit' onclick=\&quot;setTimeout(function() 
{ updateByPk('Layer2', '380118185183', 'Ed Hardy', '1'); } ),1250);\&quot; 
\/>\r\n<\/form>&quot;'); return false;">Upload files</a>

It still is not clickable..., everything seems to be quoted correctly?

When I try without htmlspecial chars, the following html output is produced:

<input type='submit' name='submit' value='Submit' onclick=" settimeout(function()="" {="" updatebypk(="" layer2="" 380118179930="" ed="" hardy="" ,="" 1="" );="" }="" ),1250);="">
'); return false;"&gt;Upload files</a>
Joshxtothe4
  • 4,061
  • 10
  • 53
  • 83
  • 1
    Paolo: If I was a cracker willing to use XSS, I would love him :) – guerda May 05 '09 at 12:34
  • 1
    @paulo: Why is my code so ugly, and what could I do to make it nicer? @guerda: How would you consider my code to be insecure? Everything is sanitized.... –  May 05 '09 at 12:36
  • @josh: Why isn't the HTML, or at least the script to generate it, already in the page somewhere else? Then the only thing your anchor does is call a function, maybe with a couple of strings to indicate what kind of pre-set thing to generate. – Anonymous May 05 '09 at 12:47
  • the html will be launched in a popup window, I put it in a variable so I can pass it to my function to create a popup. –  May 05 '09 at 12:53

3 Answers3

13

As said in the comment to the question, this is absolutely horrendous code, and you're suffering the consequences. The main problem is the number of code levels: server code that renders Javascript, that renders HTML - and difference escapes at every level and interfere with each other.

To improve the situation, have a separate PHP page with the form and have your popup link open that page - no Javascript required. If you really want to avoid having that separate page at all costs, at least have the Javascript function that generates the form in the header of the page (non-dynamic) and have the link contain only a call to that function with your variables as parameters.

Michael Borgwardt
  • 342,105
  • 78
  • 482
  • 720
  • I cannot easily have a separate php page, as my popup function uses document.write to write the html to the screen. I can put the js function in my javascript file, but the problem is still with the quotes, not that function. –  May 05 '09 at 13:41
  • I am using htmlspecialchars so the form displays, without it I get the output as per my revised question. –  May 05 '09 at 13:43
  • Yes, you can easily have a separate PHP page, because then you wouldn't have to DO all this fragile crap. The problem is with the quotes on a very superficial level, but the real problem is the EXISTENCE of a "popup function" when you can do the same thing much easier and cleaner with a separate PHP page and without using Javascript AT ALL. – Michael Borgwardt May 05 '09 at 14:05
  • How would I do this without using JavaScript? How would I create a popup, update a page after a timeout via ajax and pass variables without JavaScript? –  May 05 '09 at 14:35
  • 2
    you can create popups in pure HTML using the target="_blank" attribute in the link. You can pass variables to the target page via GET parameters. The timeout of course requires JavaScript (not sure what it's for though), as would updating the original page. I'm not saying you mustn#t use JavaScript at all, just that you seem to be overusing it for things that can be better solved in different way. – Michael Borgwardt May 05 '09 at 15:00
1

The parameter in your makewindows function ist not quoted. Your quotes are escaped (%#39). Replace it with ' and you're done.

guerda
  • 23,388
  • 27
  • 97
  • 146
  • My parameters are quoted in the php code..I am not sure what I am meant to replace. –  May 05 '09 at 12:39
  • @josh: You're talking about server-side code, he's talking about client-side code. Look at the parameters you're passing to makewindows() there in the second snippit. – Anonymous May 05 '09 at 12:46
  • I mean that the parameter is quoted server side, so it should be quoted client side? –  May 05 '09 at 12:56
  • No, it may not be quoted at all. A JavaScript function with a string parameter must get a string beginning with " or '. – guerda May 06 '09 at 07:53
1

Your ENT_QUOTES flag is screwing up the output. If you look closely you'll see that there are no actual quotes in the HTML output - just escaped entities. Make a test that doesn't use htmlspecialchars(). You should escape the quotes with a backslash OR better still add the javascript functionality unobtrusively. jQuery might help you to achieve that http://jquery.com

roborourke
  • 12,147
  • 4
  • 26
  • 37
  • jquery is not an option at the moment..., I where exactly is the quoting probling, as I am using quotes, and it worked fine until I added the setTimout code. –  May 05 '09 at 12:37
  • Fair shout - it was only a suggestion but you should definitely read about 'unobtrusive javascript' for future work. Google it when you get a chance. – roborourke May 05 '09 at 16:21