Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Vulnerability detected CWE ID 95 in version 4.2.0 #80

Closed
TatianaGarcia94 opened this issue Mar 18, 2021 · 5 comments
Closed

Vulnerability detected CWE ID 95 in version 4.2.0 #80

TatianaGarcia94 opened this issue Mar 18, 2021 · 5 comments

Comments

@TatianaGarcia94
Copy link

En la ruta: /src/Wrapper.php 1069
Nombre de la vulnerabilidad: Improper Neutralization of Directives in Dynamically Evaluated Code ('Eval Injection')

@gggeek gggeek changed the title Vulnerabilidad detectada CWE ID 95 en versión 4.2.0 Vulnerability detected CWE ID 95 in version 4.2.0 Mar 18, 2021
@gggeek
Copy link
Owner

gggeek commented Mar 18, 2021

Can you share some more details about the scanner used to generate such warning, and its configuration?
I am fully aware that the code uses the eval function, but the context is quite bounded, so there is probably no risk for your application (by default no code from the library makes uses of the Wrapper class - the only way to use it is for you to use it in your own code, or if you already have a php code-injection problem somewhere else)

@gggeek
Copy link
Owner

gggeek commented Mar 22, 2021

@TatianaGarcia94 no feedback?

@TatianaGarcia94
Copy link
Author

Estimado,
El análisis realizado fue un análisis estático, a través del cuál se detectó la línea mencionada.
Según los estándares de seguridad, se detectó esta línea como vulnerabilidad Improper Neutralization of Directives in Dynamically Evaluated Code ('Eval Injection')

@gggeek gggeek added the wontfix label Nov 24, 2022
@gggeek
Copy link
Owner

gggeek commented Nov 24, 2022

PS: The only part of the php code which is evaluated in line 1069 that comes from an untrusted source is sanitized via this call:

$opts['new_function_name'] = preg_replace(array('/\./', '/[^a-zA-Z0-9_\x7f-\xff]/'), array('_', ''), $mName);

I think that is sufficiently safe - unless there are attacks at play based which would fool preg_replace, which I have not heard about.
Note that $mName is guaranteed to be an utf8 stream, unless the developer has gone out of its way to change the value of PhpXmlRpc::$xmlrpc_internalencoding before calling wrapXmlrpcServer

@gggeek gggeek closed this as not planned Won't fix, can't repro, duplicate, stale Nov 24, 2022
@gggeek gggeek reopened this Nov 27, 2022
@gggeek gggeek removed the wontfix label Nov 27, 2022
@gggeek
Copy link
Owner

gggeek commented Nov 28, 2022

In the end, I found another place where untrusted data was used to generate php code, sadly without appropriate sanitization being applied. This has now been fixed.

Detailed explanation of the specific conditions in which this issue might be abused are in https://github.com/gggeek/phpxmlrpc/releases/tag/4.9.0

Thanks for reporting this - and sorry for taking so long to fix it. I did underestimate the reported security-related tickets because there was little information provided regarding the exact problem scenario / underlying issue, and the reports seemed to come from an automated scanner tool, run without any verification of its findings, and my own experience that leads usually to a large number of false positives.

@gggeek gggeek closed this as completed Nov 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants