Skip to content

Conversation

@ks3
Copy link
Contributor

@ks3 ks3 commented Feb 17, 2025

We have an IdP that includes eduPersonTargetedID in the asserted attributes. Here's an example of the attribute.

<saml2:Attribute FriendlyName="eduPersonTargetedID" Name="urn:oid:1.3.6.1.4.1.5923.1.1.1.10" NameFormat="urn:oasis:names:tc:SAML:2.0:attrname-format:uri">
    <saml2:AttributeValue>
        <saml2:NameID Format="urn:oasis:names:tc:SAML:2.0:nameid-format:persistent" NameQualifier="https://example.com/idp/shibboleth" SPNameQualifier="https://example.com/sp">exampletargetedid</saml2:NameID>
    </saml2:AttributeValue>
</saml2:Attribute>

The process function in AttributeAddFromLDAP.php loops over each attribute and assumes that if it has a value, it must be a string; this causes the exception below.

Feb 17 14:52:43 SimpleSAMLphp ERROR [e5cdb08de3] SimpleSAML\Error\Error: UNHANDLEDEXCEPTION
Feb 17 14:52:43 SimpleSAMLphp ERROR [e5cdb08de3] Backtrace:
Feb 17 14:52:43 SimpleSAMLphp ERROR [e5cdb08de3] 2 /var/www/simplesamlphp-2.3.5/src/SimpleSAML/Error/ExceptionHandler.php:39 (SimpleSAML\Error\ExceptionHandler::customExceptionHandler)
Feb 17 14:52:43 SimpleSAMLphp ERROR [e5cdb08de3] 1 /var/www/simplesamlphp-2.3.5/vendor/symfony/error-handler/ErrorHandler.php:538 (Symfony\Component\ErrorHandler\ErrorHandler::handleException)
Feb 17 14:52:43 SimpleSAMLphp ERROR [e5cdb08de3] 0 [builtin] (N/A)
Feb 17 14:52:43 SimpleSAMLphp ERROR [e5cdb08de3] Caused by: TypeError: strlen(): Argument #1 ($string) must be of type string, SAML2\XML\saml\NameID given
Feb 17 14:52:43 SimpleSAMLphp ERROR [e5cdb08de3] Backtrace:
Feb 17 14:52:43 SimpleSAMLphp ERROR [e5cdb08de3] 7 /var/www/simplesamlphp-2.3.5/modules/ldap/src/Auth/Process/AttributeAddFromLDAP.php:99 (strlen)
Feb 17 14:52:43 SimpleSAMLphp ERROR [e5cdb08de3] 6 /var/www/simplesamlphp-2.3.5/modules/ldap/src/Auth/Process/AttributeAddFromLDAP.php:99 (SimpleSAML\Module\ldap\Auth\Process\AttributeAddFromLDAP::process)
Feb 17 14:52:43 SimpleSAMLphp ERROR [e5cdb08de3] 5 /var/www/simplesamlphp-2.3.5/src/SimpleSAML/Auth/ProcessingChain.php:212 (SimpleSAML\Auth\ProcessingChain::processState)
Feb 17 14:52:43 SimpleSAMLphp ERROR [e5cdb08de3] 4 /var/www/simplesamlphp-2.3.5/modules/saml/src/Auth/Source/SP.php:1173 (SimpleSAML\Module\saml\Auth\Source\SP::handleResponse)
Feb 17 14:52:43 SimpleSAMLphp ERROR [e5cdb08de3] 3 [builtin] (call_user_func_array)
Feb 17 14:52:43 SimpleSAMLphp ERROR [e5cdb08de3] 2 /var/www/simplesamlphp-2.3.5/src/SimpleSAML/HTTP/RunnableResponse.php:68 (SimpleSAML\HTTP\RunnableResponse::sendContent)
Feb 17 14:52:43 SimpleSAMLphp ERROR [e5cdb08de3] 1 /var/www/simplesamlphp-2.3.5/vendor/symfony/http-foundation/Response.php:423 (Symfony\Component\HttpFoundation\Response::send)
Feb 17 14:52:43 SimpleSAMLphp ERROR [e5cdb08de3] 0 /var/www/simplesamlphp-2.3.5/public/module.php:24 (N/A)

To resolve this, I've just added an is_string check prior to calling strlen.

@tvdijen
Copy link
Member

tvdijen commented Feb 17, 2025

Thanks!

SAML attributes are essentially strings, even if they represent XML-structures..
I wonder if it's not a better solution to do strlen((string) $val[0]) > 0 ..

@tvdijen tvdijen merged commit dcac97a into simplesamlphp:master Feb 17, 2025
7 of 8 checks passed
@ks3
Copy link
Contributor Author

ks3 commented Feb 17, 2025

Thanks! Guessing it's good as is since it's been merged, but if you decide you prefer the typecast route there's one additional change required.

@@ -96,8 +96,8 @@ class AttributeAddFromLDAP extends BaseFilter
         foreach ($attributes as $attr => $val) {
             $arrSearch[] = '%' . $attr . '%';

-            if (is_array($val) && count($val) > 0 && is_string($val[0]) && strlen($val[0]) > 0) {
-                $arrReplace[] = $this->connector->escapeFilterValue($val[0], true);
+            if (is_array($val) && count($val) > 0 && strlen((string) $val[0]) > 0) {
+                $arrReplace[] = $this->connector->escapeFilterValue((string) $val[0], true);
             } else {
                 $arrReplace[] = '';
             }

@tvdijen
Copy link
Member

tvdijen commented Feb 17, 2025

I don't have a strong preference, I was just thinking out loud :)
Merged and tagged v2.4.4.. Also about to tag SSP v2.3.6

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Dec 16, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants