Security flaw justification

[Umesh Javalkar]

 

We have used telerik controls in our project for development. Veracode has scan the code dlls and found security issues. Below are few secuirty issues.

Cross-Site Scripting - Improper Neutralization of Script-Related HTML Tags in a Web Page (Basic XSS)

telerik_web_ui_dll.Telerik.Web.UI.Appointment	Appointment Clone()
telerik_web_ui_dll.Telerik.Web.UI.GridGroupPanel	bool AddGroupByFromTableView(GridTableView, int, ref int)
telerik_web_ui_dll.Telerik.Web.UI.RadAjaxManager	string RenderUserControl(string, object)
telerik_web_ui_dll.Telerik.Web.UI.GridAttachmentColumn	System.Web.UI.WebControls.IButtonControl InitializeButtonInCell(GridItem)
telerik_web_ui_dll.Telerik.Web.UI.GridAttachmentColumn	System.Web.UI.WebControls.IButtonControl InitializeButtonInCell(GridItem)
telerik_web_ui_dll.Telerik.Web.UI.GridAttachmentColumn	System.Web.UI.WebControls.IButtonControl InitializeButtonInCell(GridItem)

there are many such items ,do have any justfication to prove that that these are not a secuirty concerns?

[Veli]

Hi Umesh,

We have performed extensive security analysis with Veracode. The XSS vulnerability warnings in all the reports usually indicate values of control properties being used in the generated HTML.

Many of the RadControls provide properties, the values of which get rendered as part of the control markup. HTML-encoding these properties are an approach recommended by Veracode and we have taken the necessary measures accordingly and where possible. However, not all properties can be HTML-encoded without limiting the usability of the control. Some properties are expected to allow HTML strings and we cannot HTML-encode their values before adding them to the rendered control markup.

These properties are serialized in the page ViewState. Tampering with property values is possible only in cases when ViewState is encrypted, tampered with and serialized back to the server, where tampering goes by unnoticed by the ASP.NET security mechanisms. You will agree that attacks involving modified ViewState are beyond the scope of any particular server control and such scenarios cannot be identified and prevented at the control level.

Veli
the Telerik team

[Umesh Javalkar]

 

Another flaw has been identified and that is related to "Directory Traversal". Can you please provide a justification for this so that I can forward the same to Veracode team. Please find the details below.

Description is as follow:

Scope	Function Prototype
telerik_web_ui_dll.Telerik.Web.UI.Widgets.FileSystemContentProvider	string CopyDirectory(string, string)

This call to mscorlib_dll.System.IO.Directory.CreateDirectory() contains a path manipulation flaw. The argument to the function is a filename constructed using user-supplied input. If an attacker is allowed to specify all or part of the filename, it may be possible to gain unauthorized access to files on the server, including those outside the webroot, that would be normally be inaccessible to end users. The level of exposure depends on the effectiveness of input validation routines, if any. The first argument to CreateDirectory() contains tainted data. The tainted data originated from earlier calls to system_web_dll.system.web.ui.control.get_viewstate, and system_web_dll.system.web.httprequest.get_applicationpath.

Validate all user-supplied input to ensure that it conforms to the expected format, using centralized data validation routines when possible. When using black lists, be sure that the sanitizing routine performs a sufficient number of iterations to remove all instances of disallowed characters.

 

References:
CWE (http://cwe.mitre.org/data/definitions/73.html)
WASC (http://webappsec.pbworks.com/Path-Traversal)

telerik_web_ui_dll.Telerik.Web.UI.Dictionaries.FileCustomDictionarySource

void AddWord(string)

This call to mscorlib_dll.System.IO.StreamReader.!newinit_0_2() contains a path manipulation flaw. The argument to the function is a filename constructed using user-supplied input. If an attacker is allowed to specify all or part of the filename, it may be possible to gain unauthorized access to files on the server, including those outside the webroot, that would be normally be inaccessible to end users. The level of exposure depends on the effectiveness of input validation routines, if any. The first argument to !newinit_0_2() contains tainted data. The tainted data originated from an earlier call to system_web_dll.system.web.httprequest.get_form.

Validate all user-supplied input to ensure that it conforms to the expected format, using centralized data validation routines when possible. When using black lists, be sure that the sanitizing routine performs a sufficient number of iterations to remove all instances of disallowed characters.

References:
CWE (http://cwe.mitre.org/data/definitions/73.html)
WASC (http://webappsec.pbworks.com/Path-Traversal)

telerik_web_ui_dll.Telerik.Web.UI.RadFileExplorer

void DeleteItems(string[])

This call to telerik_web_ui_dll.Telerik.Web.UI.Widgets.FileBrowserContentProvider.DeleteFile() contains a path manipulation flaw. The argument to the function is a filename constructed using user-supplied input. If an attacker is allowed to specify all or part of the filename, it may be possible to gain unauthorized access to files on the server, including those outside the webroot, that would be normally be inaccessible to end users. The level of exposure depends on the effectiveness of input validation routines, if any. The first argument to DeleteFile() contains tainted data. The tainted data originated from earlier calls to system_web_dll.system.web.ui.control.get_viewstate, and system_web_dll.system.web.httprequest.get_params.

Validate all user-supplied input to ensure that it conforms to the expected format, using centralized data validation routines when possible. When using black lists, be sure that the sanitizing routine performs a sufficient number of iterations to remove all instances of disallowed characters.

References:
CWE (http://cwe.mitre.org/data/definitions/73.html)
WASC (http://webappsec.pbworks.com/Path-Traversal)

[Lini]

Hello,

First of all, make sure that you are using version Q2 2011 or later - our Veracode scans were done with the Q2 2011. Here are the justifications for the flaws you listed:


telerik_web_ui_dll.Telerik.Web.UI.Widgets.FileSystemContentProvider / string CopyDirectory(string, string)
The methods MoveFile,MoveDirectory,DeleteFile,DeleteDirectory,CreateDirectory,CopyFile,CopyDirectory are part of our content provider implementation that allows the file explorer control to interface with the file system on the server and enable the user to manage folders and files. There is no permissions/path check done in these methods, so they are flagged by Veracode as potential directory traversal flaws.
The actual permissions checking is done in the code that call these methods. The code we use to verify the path and permissions are correct are in the FileBrowserContentProvider base class - CheckReadPermissions, CheckDeletePermissions, CheckWritePermissions. Each of those is called when a request is made for a file operation. For example - CheckDeletePermissions is called before we call DeleteFile to make sure that the supplied path is valid and falls under the allowed set of folders. The persmissions are set on the server in the RadEditor dialogs configuration for the editor and the RadFileExplorer configuration for the file explorer.


telerik_web_ui_dll.Telerik.Web.UI.RadFileExplorer / void DeleteItems(string[])
This method actually calls functions in the FileSystemContentProvider class (DeleteDirectory and DeleteFile) so you can use the same justification.


telerik_web_ui_dll.Telerik.Web.UI.Dictionaries.FileCustomDictionarySource / void AddWord(string)
The methods open a file from a specific path. The path argument is build from a couple of properties, which are set only on the server and stored in the ViewState. There is no posibility to change the value of these properties from the browser.

Kind regards,
Lini
the Telerik team

[Umesh Javalkar]

Thanks for the quick response.
Yes, we're upgrading Telerik.Web.UI.dll to 2011.2.915.35 version and sending new build to Veracode for re-scanning. Hope all the issues listed by Veracode will get solved. In case If I require anything I'll post it.

[Umesh Javalkar]

We have upgraded to Q2 2011 (2011.2.915) and Veracode has rescanned the code. Following are the flaws left which require justification.

CRLF Injection:

Scope	Function Prototype
telerik_web_ui_dll.Telerik.Web.UI.ImageEditor.ImageEditorCacheHandler	void SendImage(EditableImage, System.Web.HttpContext, string, string)

This call to system_web_dll.System.Web.HttpResponse.AddHeader() contains an HTTP response splitting flaw. Writing unsanitized user-supplied input into an HTTP header allows an attacker to manipulate the HTTP response rendered by the browser, leading to cache poisoning and cross-site scripting attacks. The second argument to AddHeader() contains tainted data. The tainted data originated from an earlier call to system_web_dll.System.Web.HttpRequest.get_Item.

Remove unexpected carriage returns and line feeds from user-supplied data used to construct HTTP response headers. Whenever possible, use a security library such as ESAPI that provides safe versions of addHeader(), etc. that will automatically remove unexpected carriage returns and line feeds and can be configured to use HTML entity encoding for non-alphanumeric data. Only write custom blacklisting code when absolutely necessary. Always validate user-supplied input to ensure that it conforms to the expected format, using centralized data validation routines when possible.

References:
CWE (http://cwe.mitre.org/data/definitions/113.html)
OWASP (http://www.owasp.org/index.php/HTTP_Response_Splitting)

telerik_web_ui_dll.Telerik.Web.UI.ImageEditor.ImageEditorCacheHandler	void WriteFile(byte[], string, string, System.Web.HttpResponse)
telerik_web_ui_dll.Telerik.Web.UI.ImageEditor.ImageEditorCacheHandler	void WriteFile(byte[], string, string, System.Web.HttpResponse)

This call to system_web_dll.System.Web.HttpResponse.AddHeader() contains an HTTP response splitting flaw. Writing unsanitized user-supplied input into an HTTP header allows an attacker to manipulate the HTTP response rendered by the browser, leading to cache poisoning and cross-site scripting attacks. The second argument to AddHeader() contains tainted data. The tainted data originated from an earlier call to system_web_dll.System.Web.HttpRequest.get_Item.

Remove unexpected carriage returns and line feeds from user-supplied data used to construct HTTP response headers. Whenever possible, use a security library such as ESAPI that provides safe versions of addHeader(), etc. that will automatically remove unexpected carriage returns and line feeds and can be configured to use HTML entity encoding for non-alphanumeric data. Only write custom blacklisting code when absolutely necessary. Always validate user-supplied input to ensure that it conforms to the expected format, using centralized data validation routines when possible.

References:
CWE (http://cwe.mitre.org/data/definitions/113.html)
OWASP (http://www.owasp.org/index.php/HTTP_Response_Splitting)

Cryptographic Issues

Insufficient Entropy

Standard random number generators do not provide a sufficient amount of entropy when used for security purposes. Attackers can brute force the output of pseudorandom number generators such as rand().

If this random number is used where security is a concern, such as generating a session key or session identifier, use a trusted cryptographic random number generator instead. These can be found on the Windows platform in the CryptoAPI or in an open source library such as OpenSSL. In Java, use the SecureRandom object to ensure sufficient entropy.

References:
CWE (http://cwe.mitre.org/data/definitions/331.html)

Code Quality

Improper Resource Shutdown or Release

telerik_web_ui_dll.Telerik.Web.Apoc.Render.Xml.XMLRenderer

void StartRenderer()

There are total of 1 instances. The program fails to release or incorrectly releases some variables, e.g. the variable ms, which was previously allocated by a call to mscorlib_dll.System.IO.MemoryStream.!newinit_0_0().

Ensure that all code paths properly release this resource.

References:
CWE (http://cwe.mitre.org/data/definitions/404.html)

Information Leakage	Information Exposure Through an Error Message

The application calls the system_dll.System.Net.WebRequest.GetResponse() function, which will result in data being transferred out of the application (via the network or another medium). This data contains sensitive information. GetResponse() was called on an object, which contains potentially sensitive data. The potentially sensitive data originated from an earlier call to mscorlib_dll.System.IO.FileStream.!ctor.

Ensure that the transfer of the sensitive data is intended and that it does not violate application security policy.

References:
CWE (http://cwe.mitre.org/data/definitions/201.html)
WASC (http://webappsec.pbworks.com/Information-Leakage)

The application calls the system_web_dll.System.Web.HttpResponse.Write() function, which may expose information about the application logic or other details such as the names and versions of the application container and associated components. This information can be useful in executing other attacks and can also enable the attacker to target known vulnerabilities in application components. The first argument to Write() contains potentially sensitive data. The potentially sensitive data originated from an earlier call to mscorlib_dll.System.Exception.get_Message. The potentially sensitive data is directed into an output stream returned by system_web_dll.System.Web.UI.Page.

Ensure that error codes or other messages returned to end users are not overly verbose. Sanitize all messages of any sensitive information that is not absolutely necessary.

References:
CWE (http://cwe.mitre.org/data/definitions/209.html)

Information Leakage	Information Exposure Through Sent Data
Information Leakage	Information Exposure Through an Error Message

telerik_web_ui_dll.Telerik.Web.UI.Editor.DialogControls.HTTPSend
telerik_web_ui_dll.Telerik.Web.UI.Editor.DialogControls.CSDialog

void GetResponse()
void OnLoad(System.EventArgs)

The application calls the system_dll.System.Net.WebRequest.GetResponse() function, which will result in data being transferred out of the application (via the network or another medium). This data contains sensitive information. GetResponse() was called on an object, which contains potentially sensitive data. The potentially sensitive data originated from an earlier call to mscorlib_dll.System.IO.FileStream.!ctor.

Ensure that the transfer of the sensitive data is intended and that it does not violate application security policy.

References:
CWE (http://cwe.mitre.org/data/definitions/201.html)
WASC (http://webappsec.pbworks.com/Information-Leakage)

The application calls the system_web_dll.System.Web.HttpResponse.Write() function, which may expose information about the application logic or other details such as the names and versions of the application container and associated components. This information can be useful in executing other attacks and can also enable the attacker to target known vulnerabilities in application components. The first argument to Write() contains potentially sensitive data. The potentially sensitive data originated from an earlier call to mscorlib_dll.System.Exception.get_Message. The potentially sensitive data is directed into an output stream returned by system_web_dll.System.Web.UI.Page.

Ensure that error codes or other messages returned to end users are not overly verbose. Sanitize all messages of any sensitive information that is not absolutely necessary.

References:
CWE (http://cwe.mitre.org/data/definitions/209.html)

[Lini]

Hi,

Here are the justifications for the remaining issues:


telerik_web_ui_dll.Telerik.Web.UI.ImageEditor.ImageEditorCacheHandler.SendImage(EditableImage, System.Web.HttpContext, string, string)

The CRLF flaw was fixed in the Q2 2011 SP1 release. Here is the applicable code:

fileName = string.IsNullOrEmpty(fileName) ? "Telerik_RadImageEditor_Image" : fileName.Replace("\n", " ").Replace("\r", " ");
context.Response.AddHeader("content-disposition", string.Format("attachment; filename={0}.{1}", fileName, editableImage.Format));

telerik_web_ui_dll.Telerik.Web.UI.ImageEditor.ImageEditorCacheHandler.WriteFile(byte[], string, string, System.Web.HttpResponse)

The CRLF flaw was fixed in the Q2 2011 SP1 release. Here is the applicable code:

fileName = fileName.Replace("\n", " ").Replace("\r", " ");
contentType = contentType.Replace("\n", " ").Replace("\r", " ");

response.Buffer = true;
response.Clear();

response.ContentType = contentType;
response.AddHeader("content-disposition", "attachment; filename=" + fileName);

telerik_web_ui_dll.Telerik.Web.Apoc.Render.Xml.XMLRenderer.StartRenderer()

The resources are released in another method - StopRenderer(), which is always called after StartRenderer()


telerik_web_ui_dll.Telerik.Web.UI.Editor.DialogControls.HTTPSend.GetResponse()

No remediation necessary - the flaw is only informational. The error message needs to be shown as it is a part of our integration with the W3C Validator application.


telerik_web_ui_dll.Telerik.Web.UI.Editor.DialogControls.CSDialog.OnLoad(System.EventArgs)

No remediation necessary - the flaw is only informational. The error message needs to be shown as it is a part of our integration with HiSoftware's Compliance Sheriff application.



All the best,
Lini
the Telerik team