Re: [PATCH] Two bugs in URL parsing from commit c419542d (bug #68295, #68296)

Tim Rühsen <[email protected]> Wed, 17 Jun 2026 14:27:36 +0200
Newsgroups gmane.comp.web.wget.general
Message-ID <[email protected]>
Thank you, Samuel!

Just reviewed/tested your patches (both lgtm) and pushed them to the=20
Gitlab repository (https://gitlab.com/gnuwget/wget).
Currently, I don't have my Savannah credentials around, but take care=20
for updating Savannah asap.

Adding Darshit to CC (just in case you are going to push anything to=20
Savannah soon).

Regards, Tim

On 6/16/26 11:50 PM, Dainard, Samuel via Primary discussion list for GNU=
=20
Wget wrote:
> Hi,
>
> I filed two related bug reports on Savannah about seven weeks ago for is=
sues introduced in commit c419542d. Both have patches attached but haven't=
 gotten any response yet. Known wget is high priority so hope y'all don't =
mind me sending them to the list for visibility.
>
> Bug #68295 - is_valid_port: undefined behavior with atoi on overflow
>    https://savannah.gnu.org/bugs/?68295 (https://savannah.gnu.org/bugs/?=
68295)
>
> is_valid_port() uses atoi() to parse the port string. Since atoi() has u=
ndefined behavior on integer overflow, a string like "99999999999" trigger=
s UB before the range check can reject it. The patch replaces atoi() with =
strtoul(), which sets errno on overflow and whose end pointer eliminates t=
he separate strspn() call.
>
> Bug #68296 - maybe_prepend_scheme: strchr finds colons in path component
>    https://savannah.gnu.org/bugs/?68296 (https://savannah.gnu.org/bugs/?=
68296)
>
> maybe_prepend_scheme() uses strchr(url, ':') which matches colons anywhe=
re in the URL, including the path. A schemeless URL like www.example.com/p=
ath:name (http://www.example.com/path:name)<http://www.example.com/path:na=
me%20(http:/www.example.com/path:name)> hits the colon in "path:name", is_=
valid_port("name") returns false, and the function refuses to prepend http=
://. The patch adds a slash check so the colon search is limited to the au=
thority component, restoring the behavior of the previous strpbrk(url, ":/=
") approach without reintroducing removed shorthand URL support.
>
> Patches are on the respective bug reports. Happy to rebase or rework if =
needed.
>
> Thanks,
> Samuel Dainard
> sdainard@
>