phpxmlrpc icon indicating copy to clipboard operation
phpxmlrpc copied to clipboard

Vulnerability detected CWE ID 209 in version 4.2.0

Open TatianaGarcia94 opened this issue 3 years ago • 1 comments

En la ruta: /src/Request.php 304 Nombre de la vulnerabilidad: Generation of Error Message Containing Sensitive Information Modo de detección: Se realizó un escaneo estático, el cuál detecto la línea mencionada como vulnerabilidad según los estándares de seguridad

TatianaGarcia94 avatar Mar 24 '21 03:03 TatianaGarcia94

Thanks a lot for taking time to run this analysis and posting the resulting issues.

However, as with any automated security scanning tool, there are risks of generating false positives (as well as false negatives of course). Best results are generally obtained when the tool is configured by a security expert in collaboration with a domain expert, and, even then, they are not to be taken at face value, but should be subject to in-depth human analysis in order for the severity and impact to be properly assessed.

In the case of this ticket, there are two easily visible reasons why I am not very concerned, but rather inclined to dismiss it as 'not an issue':

  1. you are reporting bugs against version 4.2.0. The current version of the lib is 4.6.0, and there is not a lot of reason to stay on 4.2.0. Even if you follow strict semantic-versioning rules, you should upgrade at least to 4.2.2, released on Oct 2017.
  2. the particular line which is echoing a detailed error message is wrapped inside an if the developer has enabled debugging mode condition. Debug mode is not turned on by default, but has to be explicitly. This is imho a case of the scanner tool not being intelligent enough to understand that and creating a false positive.

gggeek avatar Mar 24 '21 09:03 gggeek

PS: even with debug mode on, and if the code is being run within a browser, the way the error string which is being printed is built makes it safe, as "not subject to html injection". The only possible attack is if the php function xml_error_string does return an unsafe text. Which I am pretty sure it never does, given that the error messages it returns are enumerable

gggeek avatar Nov 24 '22 18:11 gggeek

PS: I double-checked the current version of the code. The equivalent line is now at Request.php:345

print $xmlRpcParser->_xh['isf_reason']

By looking at how the isf_reason value is built, one can see that the only untrusted data which is used in it is the name of an xml element, as gotten from the xml parser used by php. Assuming the parser is not vulnerable to attacks, it should reject anything which is not compliant with the XML spec, which has this to say about element names: https://www.w3.org/TR/xml/#NT-Name

gggeek avatar Nov 24 '22 19:11 gggeek