3
14. public void LoadData() 

16. plan = [Select id, name,Icon__c,Bar_color__c from Activitytype__c limit 20]; 

41. <div Style="background-color:#{!row.Bar_color__c};padding-bottom:3px;padding-top:3px;padding-left:20px;padding-right:20px; height:24px;width:200px;display:inline;"> 

7. public List<Activitytype__c> plan get;set;} 

41. <div Style="background-color:#{!row.Bar_color__c};padding-bottom:3px;padding-top:3px;padding-left:20px;padding-right:20px; height:24px;width:200px;display:inline;"> 

65. <type>Text</type>

41. <div Style="background-color:#{!row.Bar_color__c};padding-bottom:3px;padding-top:3px;padding-left:20px;padding-right:20px; height:24px;width:200px;display:inline;"> 
3
  • please can any one give an idea about how to fix this type of errors???
    – fsales572
    Commented Jul 29, 2014 at 6:22
  • for remaining lines in above code 14,16,7,65 which changes i do?
    – fsales572
    Commented Jul 29, 2014 at 6:40
  • In my experience, checkmarx stops complaining as soon as you have wrapped {!row.Bar_color__c} with HTMLENCODE. The other lines (14,16,7,65) are there primarily to show you the execution sequence and need not necessarily all to be changed. If your goal is not only to keep checkmarx quiet but to make it right, then you should really consider all the aspects of @sfdcfox 's answer. Without HTMLENCODE, checkmarx will go on to complain - at least it did so in my projects. The good news is, that the code above will not break by HTMLENCODE you can just use it and it's really an improvement.
    – Uwe Heim
    Commented Jul 29, 2014 at 11:15

2 Answers 2

1

You can wrap all the occurrences of the dynamic color in your markup like this {!HTMLENCODE(row.Bar_color__c)}

1

You're allowing a user to inject whatever code they want into the Bar_Color__c-- even a picklist is not a guarantee that it will be a legal value, since users can use the API to insert whatever value they want. At minimum, you should check to make sure that Bar_Color__c is from a list of allowed colors or matches a pattern that represents a legal CSS color, such as a rgb tuple (rgb(num, num, num) or a list of defined colors (black, white, pink, etc). Or, you should at least use HTMLENCODE so that quotation marks can't be abused, but this would still allow for invalid colors.

2
  • Hi @sfdcfox, there is a preceeding #, so I guess he is only going for the hex-notation. Therefore a easy regex could be a good measure.
    – Uwe Heim
    Commented Jul 29, 2014 at 11:21
  • I missed this comment originally, but while you're mostly right, one could put in a closing style tag followed by malicious JavaScript. Not much, since fields are limited to 255 characters, but enough to do some harm.
    – sfdcfox
    Commented Dec 2, 2014 at 6:34

You must log in to answer this question.

Not the answer you're looking for? Browse other questions tagged .