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

Undefined index: errors in UriString.php on line 455 & other errors #21

Closed
ntzm opened this issue Jan 14, 2020 · 9 comments · Fixed by thephpleague/uri#151
Closed

Undefined index: errors in UriString.php on line 455 & other errors #21

ntzm opened this issue Jan 14, 2020 · 9 comments · Fixed by thephpleague/uri#151

Comments

@ntzm
Copy link
Contributor

ntzm commented Jan 14, 2020

Bug Report

Information Description
Version 6.0.1
PHP version 7.4.1
OS Platform Arch Linux

Summary

Standalone code, or other way to reproduce the problem

<?php

require __DIR__ . '/vendor/autoload.php';

\League\Uri\Uri::createFromString('//:�@�����������������������������������������������������������������������������������������/');

Expected result

Parse error

Actual result

Throws:

PHP Notice:  Undefined index: errors in /home/nat/Code/personal/fuzzer-test/vendor/league/uri/src/UriString.php on line 455

Notice: Undefined index: errors in /home/nat/Code/personal/fuzzer-test/vendor/league/uri/src/UriString.php on line 455
PHP Notice:  Undefined index: errors in /home/nat/Code/personal/fuzzer-test/vendor/league/uri/src/UriString.php on line 456

Notice: Undefined index: errors in /home/nat/Code/personal/fuzzer-test/vendor/league/uri/src/UriString.php on line 456
PHP Fatal error:  Uncaught TypeError: Argument 1 passed to League\Uri\UriString::getIDNAErrors() must be of the type int, null given, called in /home/nat/Code/personal/fuzzer-test/vendor/league/uri/src/UriString.php on line 456 and defined in /home/nat/Code/personal/fuzzer-test/vendor/league/uri/src/UriString.php:477
Stack trace:
#0 /home/nat/Code/personal/fuzzer-test/vendor/league/uri/src/UriString.php(456): League\Uri\UriString::getIDNAErrors(NULL)
thephpleague/uri#1 /home/nat/Code/personal/fuzzer-test/vendor/league/uri/src/UriString.php(390): League\Uri\UriString::filterRegisteredName('\xEF\xBF\xBD\xEF\xBF\xBD\xEF\xBF\xBD\xEF\xBF\xBD\xEF\xBF\xBD...')
thephpleague/uri#2 /home/nat/Code/personal/fuzzer-test/vendor/league/uri/src/UriString.php(351): League\Uri\UriString::filterHost('\xEF\xBF\xBD\xEF\xBF\xBD\xEF\xBF\xBD\xEF\xBF\xBD\xEF\xBF\xBD...')
thephpleague/uri#3 /home/nat/Code/personal/fuzzer-test/vendor/league/uri/src/UriString.php(314): League\Uri\UriString::parseAuthority(':\xEF\xBF\xBD@\xEF\xBF\xBD\xEF\xBF\xBD\xEF\xBF\xBD\xEF...')
thephpleague/uri#4 /home/nat/Code/personal/fuzzer-te in /home/nat/Code/personal/fuzzer-test/vendor/league/uri/src/UriString.php on line 477

Fatal error: Uncaught TypeError: Argument 1 passed to League\Uri\UriString::getIDNAErrors() must be of the type int, null given, called in /home/nat/Code/personal/fuzzer-test/vendor/league/uri/src/UriString.php on line 456 and defined in /home/nat/Code/personal/fuzzer-test/vendor/league/uri/src/UriString.php:477
Stack trace:
#0 /home/nat/Code/personal/fuzzer-test/vendor/league/uri/src/UriString.php(456): League\Uri\UriString::getIDNAErrors(NULL)
thephpleague/uri#1 /home/nat/Code/personal/fuzzer-test/vendor/league/uri/src/UriString.php(390): League\Uri\UriString::filterRegisteredName('\xEF\xBF\xBD\xEF\xBF\xBD\xEF\xBF\xBD\xEF\xBF\xBD\xEF\xBF\xBD...')
thephpleague/uri#2 /home/nat/Code/personal/fuzzer-test/vendor/league/uri/src/UriString.php(351): League\Uri\UriString::filterHost('\xEF\xBF\xBD\xEF\xBF\xBD\xEF\xBF\xBD\xEF\xBF\xBD\xEF\xBF\xBD...')
thephpleague/uri#3 /home/nat/Code/personal/fuzzer-test/vendor/league/uri/src/UriString.php(314): League\Uri\UriString::parseAuthority(':\xEF\xBF\xBD@\xEF\xBF\xBD\xEF\xBF\xBD\xEF\xBF\xBD\xEF...')
thephpleague/uri#4 /home/nat/Code/personal/fuzzer-te in /home/nat/Code/personal/fuzzer-test/vendor/league/uri/src/UriString.php on line 477

Found with https://github.com/nikic/PHP-Fuzzer

@nyamsprod
Copy link
Member

@ntzm nice catch 👍 could you share the complete setup you use to get this error ? I'm interested by this, I might found other bugs this way. Either way I'll try to fix this or you can also submit a PR for that if you got the time.

@ntzm
Copy link
Contributor Author

ntzm commented Jan 14, 2020

@ntzm
Copy link
Contributor Author

ntzm commented Jan 14, 2020

I will submit a PR shortly

@ntzm
Copy link
Contributor Author

ntzm commented Jan 14, 2020

The error happens because idn_to_ascii returns false on the dodgy input, however https://github.com/thephpleague/uri/blob/master/src/UriString.php#L460-L462 indicates that it believes the Intl extension is misconfigured. Although the two if statements need swapping round, I don't think my intl extension is misconfigured.

@nyamsprod
Copy link
Member

yes indeed the two ifs do need swapping it should resolve the issue 👍

ntzm referenced this issue in ntzm/uri Jan 14, 2020
@nyamsprod
Copy link
Member

nyamsprod commented Jan 14, 2020

@ntzm now I remember why the order was what it was.... check this
https://3v4l.org/ad9SK

basically if we want to correctly cover our grounds we need to check the last parameter populated by reference $info in the example

  • if it's an empty array <- it is an error
  • else if the 'error' index is different to 0 we have a documented error
  • else the input is correct.

What do you think ?

@ntzm
Copy link
Contributor Author

ntzm commented Jan 14, 2020

Makes sense, will update

@nyamsprod
Copy link
Member

I think we might in this case even add a comment inside the code so that we don't forget why the check is made this way. Otherwise futur me/us will find this check strange and may introduce a bug while thinking he/she his fixing an inconsistency 😄

@nyamsprod
Copy link
Member

anyway I'll review your updated PR in time thanks for the work 👍

ntzm referenced this issue in ntzm/uri Jan 14, 2020
nyamsprod referenced this issue in thephpleague/uri Jan 15, 2020
@nyamsprod nyamsprod transferred this issue from thephpleague/uri Nov 17, 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

Successfully merging a pull request may close this issue.

2 participants