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

ReldepParser: Reject overlong dependency strings #268

Merged
merged 1 commit into from
Feb 8, 2023

Conversation

ppisar
Copy link
Contributor

@ppisar ppisar commented Feb 6, 2023

Passing a very long package name triggered a segfault:

# dnf5 downgrade $(perl -e 'print q{A} x 13390')
Updating and loading repositories:
Repositories loaded.
Segmentation fault (core dumped)

This is caused by a known bug in GCC std::regex_match().

This patch prevents from the crash by rejecting strings longer than 1 KB. Another solution would be to move to a heap-based regular expression engine, like PCRE2.

https://bugzilla.redhat.com/show_bug.cgi?id=2164792 https://gcc.gnu.org/bugzilla/show_bug.cgi?id=86164

@j-mracek
Copy link
Contributor

j-mracek commented Feb 8, 2023

I am really sorry but I am unable to reproduce the issue with dnf5-5.0.5-20230207005715.11.g3388f6fc.fc36.x86_64. Also I don't believe that dnf5 downgrade command uses regex at all. @ppisar May I ask you to verify the reproducer?

Passing a very long package name triggered a segfault:

    # dnf5 downgrade $(perl -e 'print q{A} x 13390')
    Updating and loading repositories:
    Repositories loaded.
    Segmentation fault (core dumped)

This is caused by a known bug in GCC std::regex_match().

This patch prevents from the crash by rejecting strings longer than
1 KB. Another solution would be to move to a heap-based regular
expression engine, like PCRE2.

<https://bugzilla.redhat.com/show_bug.cgi?id=2164792>
<https://gcc.gnu.org/bugzilla/show_bug.cgi?id=86164>
@ppisar
Copy link
Contributor Author

ppisar commented Feb 8, 2023

It happens both with downgrade and with install. The exact length when the bug triggers varies based how the code is compiled (compiler version, compiler options). I can reproduce it with a code built from the latest git tree (commit 3092cd6), that build requires a longer argument:

# /tmp/build/dnf5/dnf5 upgrade $(perl -e 'print q{A} x 53390')
Updating and loading repositories:
 systemd                                                       100% |   0.0   B/s |   3.0 KiB |  00m00s
Repositories loaded.
Segmentation fault (core dumped)

Running it later under debugger and listing outmost 16 frames confirms it's the same crash.

Where can I found dnf5-5.0.5-20230207005715.11.g3388f6fc.fc36.x86_64? Fedora has dnf5-5.0.5-1.fc38.x86_64.

@ppisar
Copy link
Contributor Author

ppisar commented Feb 8, 2023

I can see you pushed new commits, I will push a rebased patch.

@kontura
Copy link
Contributor

kontura commented Feb 8, 2023

Where can I found dnf5-5.0.5-20230207005715.11.g3388f6fc.fc36.x86_64? Fedora has dnf5-5.0.5-1.fc38.x86_64.

That should be from our nightly Copr: https://copr.fedorainfracloud.org/coprs/rpmsoftwaremanagement/dnf5-unstable/

@ppisar
Copy link
Contributor Author

ppisar commented Feb 8, 2023

The same crash with unstable dnf5:

# dnf5 install $(perl -e 'print q{A} x 53390') 
Updating and loading repositories:
Repositories loaded.
Segmentation fault (core dumped)
root@fedora-38:~ # rpm -q dnf5
dnf5-5.0.5-20230208005815.21.g3092cd66.fc38.x86_64

@j-mracek
Copy link
Contributor

j-mracek commented Feb 8, 2023

I confirm that it is reproducible.

@j-mracek j-mracek self-assigned this Feb 8, 2023
Copy link
Contributor

@j-mracek j-mracek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@j-mracek j-mracek merged commit 10f28bd into rpm-software-management:main Feb 8, 2023
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 this pull request may close these issues.

3 participants