symfony / symfony

The Symfony PHP framework
https://symfony.com
MIT License
29.47k stars 9.41k forks source link

[Validator] IBAN validator does not do account number check, resulting in false positives #27888

Closed ostrolucky closed 5 years ago

ostrolucky commented 5 years ago

Symfony version(s) affected: all

Description
Current IBAN validator uses general IBAN checksum only, while some countries use internal check digit algorithms to validate domestic BBAN. What this means is that some invalid IBANs pass the Symfony validation.

How to reproduce
following should return > 0, instead it returns 0

use Symfony\Component\Validator\Constraints\Iban;
use Symfony\Component\Validator\Constraints\IbanValidator;
use Symfony\Component\Validator\Validation;

echo count(Validation::createValidator()->validate('SK1211115351562002977968', [new Iban]));

Possible Solution
Unfortunately I wasn't successful in finding these country specific algorithms, help needed.

Additional context
https://www.iban.com/iban-validation-help.html provides some basic info about this https://www.ibancalculator.com/iban_validieren.html First result in google for iban validation, it does these country specific validations too, but they sell API for this service.

javiereguiluz commented 5 years ago

@ostrolucky thanks for reporting this issue. I'd love to see this fixed, but I guess it's impossible. Just read these two sentences from the website you linked:

Every country uses a different algorithm and in some countries algorithms vary from bank to bank or even individual branches.

Germany (DE) has a complex structure with 143 algorithms.

That's where I stopped reading. I'm afraid we can't implement 143 algorithms to validate German bank accounts ... and then do the same with the other 200 countries in the world 😢

That's why I recommend to close this as "won't fix". After all, here we're validating the IBAN and that validation is correct. Some internal parts, which are not IBAN but local bank accounts, may be wrong ... but that's out of the scope of the IBAN validator.

xabbuh commented 5 years ago

I think I agree with Javier. In a real application being able to validate the structure of an IBAN is good as it allows to perform some basic checks without having to contact another service. After that you probably always have to use some third-party anyway to not have to store the IBAN as is and this service will do the required integrity checks.

ostrolucky commented 5 years ago

Remember that website sells that API and it's in their interest to scare you off. Number is probably exaggerated by telling you m^n possible paths, whereas in real source code it would probably be just a list where each row has 2-3 rules.

Rules used for these numbers are all very similar. There might be many of them, but most of them are length and MODx based and can be reused across different countries.

Of course, it's not realistic to have support for all countries from the start. At the start, there should be just one country - I can supply rule for SK IBAN. As I have found out, there is just single rule there. Volunteers can later supply rules for their countries.

After that you probably always have to use some third-party anyway to not ha ve to store the IBAN as is and this service will do the required integrity checks.

In my case (and I believe in many others too) at this point it's too late though. I maintain accommodation system for dormitory. When student is unaccommodated, their deposit is returned. These returns are done once a day. If invalid IBAN passes validation, this presents a problem, because once student is unaccommodated, there is often no way to reach them, especially if it's student from abroad. So we find out only 1-3 days later and additionally, whole payment transaction fails because of single invalid IBAN, so other students don't get deposit back until this is fixed. And office clerks are frustrated. Point of validation is that error is caught at the point of inputting it, so this situation is prevented to happen.

Finally, I admit I don't use symfony constraint for IBAN validation, but iban.js only. That's where I noticed no OSS IBAN validator has these rules implemented which is a shame. So that's why I created this issue. Even though I don't use this constraint, it would be nice if there was somebody in OSS to support this and Symfony might be good candidate. But if this is considered out of scope of built in constraint I understand. Maybe somebody should create StricterIban constraint outside of symfony and it could be considered to merge into symfony later.

ro0NL commented 5 years ago

That's where I noticed no OSS IBAN validator has these rules implemented which is a shame.

https://github.com/zendframework/zend-validator/blob/master/src/Iban.php#L68 :thinking:

edit: never mind, SF does the same thing :sweat:

edit2: see https://github.com/globalcitizen/php-iban#comparison-of-php-iban-libraries which looks very promising.

ostrolucky commented 5 years ago

All right I'm going to close this. php-iban project aims to solve this. It's horribly written, but it's not worth the effort to duplicate the work. Once it gets mature enough, then we can reimplement it in symfony constraint. If somebody is interested to start such work earlier, I'm ok to re-open this.

poolerMF commented 4 years ago

I tried php-iban, but I don't think it's working good SK91 8330 0000 0023 0067 2100 is invalid by https://www.ibancalculator.com/iban_validieren.html

php-iban says it's valid

ostrolucky commented 4 years ago

then report it to php-iban