ext/curl: accept SensitiveParameterValue in curl_setopt - #22960
Conversation
This makes it possible to pass things to curl_setopt without them leaking in backtraces.
| zval *inner_value; | ||
|
|
||
| inner_value = zend_read_property_ex( | ||
| zend_ce_sensitive_parameter_value, |
There was a problem hiding this comment.
| zend_ce_sensitive_parameter_value, | |
| Z_OBJCE_P(zvalue), |
| if (Z_TYPE_P(zvalue) == IS_OBJECT | ||
| && instanceof_function(Z_OBJCE_P(zvalue), zend_ce_sensitive_parameter_value)) | ||
| { | ||
| zval rv; |
There was a problem hiding this comment.
This theoretically leaks rv. But we know that SensitiveParameterValue is final and has a real backing store for the property and &rv is unused. I would therefore suggest to just pass NULL instead of &rv.
| zend_long lval; | ||
|
|
||
| if (Z_TYPE_P(zvalue) == IS_OBJECT | ||
| && instanceof_function(Z_OBJCE_P(zvalue), zend_ce_sensitive_parameter_value)) |
There was a problem hiding this comment.
No need for instanceof, SensitiveParameterValue is final. You can just compare the CE directly.
|
@TimWolla What do you think about this conceptually? So far, SensitiveParameterValue was only created during backtraces. Is it meant to use in user code to pass parameters? Should passing SensitiveParameterValue be supported more generally than only curl_setopt? Should all functions transparently accept SensitiveParameterValue? That would also increase the risk that the content leaks. |
I think in this instance it is quite nice as a workaround for a terrible API. I would not make this a generic thing though (e.g. the unwrapping is likely going to be expensive for type checking). Probably deserves a short mailing list discussion, though. |
2 participants
This makes it possible to pass things to curl_setopt without them leaking in backtraces.