ximion / appstream

Tools and libraries to work with AppStream metadata
http://www.freedesktop.org/wiki/Distributions/AppStream/
GNU Lesser General Public License v2.1
215 stars 114 forks source link

Crash in AppStream::Component::toString #528

Closed mss closed 1 year ago

mss commented 1 year ago

I originally reported this to KDE as bug 474402 but this looks like an upstream bug: Whatever their tool is doing wrong or however the metadata is malformed, the library shouldn't crash.

To reproduce this:

  1. Download a current KDE Neon User Edition ISO ISO, put it on an USB stick and boot
  2. Open Konsole or any other terminal you prefer
  3. Execute drkonqi-pk-debug-installer /usr/bin/dolphin (or choose any other KDE app or lib as the argument)

This will result in a crash with the following stack trace:

Application: drkonqi-pk-debug-installer (drkonqi-pk-debug-installer), signal: Segmentation fault

[KCrash Handler]
#4  0x00007fb701fc640e in AppStream::Component::toString (this=this@entry=0x0) at ../qt/component.cpp:731
#5  0x0000556d4755054e in DebugRepoEnabler::run (this=0x7ffd007f12e0) at ./src/DebugRepoEnabler.cpp:43
#6  0x0000556d475519a3 in DebugRepoEnabler::qt_metacall (this=0x7ffd007f12e0, _c=QMetaObject::InvokeMetaMethod, _id=1, _a=0x7ffd007eee20) at ./obj-x86_64-linux-gnu/src/drkonqi-pk-debug-installer_autogen/EWIEGA46WW/moc_DebugRepoEnabler.cpp:212
#7  0x00007fb701702cd5 in QQmlObjectOrGadget::metacall (this=this@entry=0x7ffd007ef060, type=type@entry=QMetaObject::InvokeMetaMethod, index=<optimized out>, argv=argv@entry=0x7ffd007eee20) at qml/qqmlobjectorgadget.cpp:51
#8  0x00007fb7015db86e in CallMethod (callType=QMetaObject::InvokeMetaMethod, callArgs=0x7fb6e41bf580, engine=0x556d490d48e0, argTypes=0x0, argCount=0, returnType=43, index=<optimized out>, object=...) at jsruntime/qv4qobjectwrapper.cpp:1303
#9  CallPrecise (object=..., data=..., engine=engine@entry=0x556d490d48e0, callArgs=callArgs@entry=0x7fb6e41bf580, callType=callType@entry=QMetaObject::InvokeMetaMethod) at jsruntime/qv4qobjectwrapper.cpp:1557
#10 0x00007fb7015dec0a in CallOverloaded (callType=<optimized out>, propertyCache=<optimized out>, callArgs=<optimized out>, engine=<optimized out>, data=..., object=...) at jsruntime/qv4qobjectwrapper.cpp:1629
#11 QV4::QObjectMethod::callInternal (this=<optimized out>, thisObject=<optimized out>, argv=<optimized out>, argc=<optimized out>) at jsruntime/qv4qobjectwrapper.cpp:2117
#12 0x00007fb7015fc253 in QV4::FunctionObject::call (argc=<optimized out>, argv=<optimized out>, thisObject=<optimized out>, this=<optimized out>) at ../../include/QtQml/5.15.10/QtQml/private/../../../../../src/qml/jsruntime/qv4functionobject_p.h:202
#13 QV4::Moth::VME::interpret (frame=0x7ffd007ef320, engine=0x556d490d48e0, code=0x7fb6e41bf538 "@\336\311\317\266\177") at jsruntime/qv4vme_moth.cpp:757
#14 0x00007fb7015fef5f in QV4::Moth::VME::exec (frame=frame@entry=0x7ffd007ef320, engine=engine@entry=0x556d490d48e0) at jsruntime/qv4vme_moth.cpp:466
#15 0x00007fb701590e3e in QV4::Function::call (this=this@entry=0x556d4920e050, thisObject=<optimized out>, argv=argv@entry=0x7fb6e41bf500, argc=<optimized out>, context=<optimized out>) at jsruntime/qv4function.cpp:69
#16 0x00007fb70171db15 in QQmlJavaScriptExpression::evaluate (this=this@entry=0x556d4932fd90, callData=callData@entry=0x7fb6e41bf4d0, isUndefined=isUndefined@entry=0x0) at qml/qqmljavascriptexpression.cpp:212
#17 0x00007fb7016ce55b in QQmlBoundSignalExpression::evaluate (this=<optimized out>, a=<optimized out>) at ../../include/QtQml/5.15.10/QtQml/private/../../../../../src/qml/jsruntime/qv4jscall_p.h:95
#18 0x00007fb7016cfc98 in QQmlBoundSignal_callback (e=0x556d4938bb20, a=0x0) at ../../include/QtQml/5.15.10/QtQml/private/../../../../../src/qml/qml/qqmlboundsignalexpressionpointer_p.h:69
#19 0x00007fb701702785 in QQmlNotifier::emitNotify (endpoint=<optimized out>, a=0x0) at qml/qqmlnotifier.cpp:104
#20 0x00007fb7010b84ab in doActivate<false> (sender=0x556d49304970, signal_index=3, argv=0x0) at kernel/qobject.cpp:3817
#21 0x00007fb7010b19f7 in QMetaObject::activate (sender=sender@entry=0x556d49304970, m=m@entry=0x7fb7018b4cc0 <QQmlComponentAttached::staticMetaObject>, local_signal_index=local_signal_index@entry=0, argv=argv@entry=0x0) at kernel/qobject.cpp:3985
#22 0x00007fb7016c0f84 in QQmlComponentAttached::completed (this=this@entry=0x556d49304970) at .moc/moc_qqmlcomponentattached_p.cpp:148
#23 0x00007fb701733cfc in QQmlObjectCreator::finalize (this=0x556d490cfaa0, interrupt=...) at qml/qqmlobjectcreator.cpp:1441
#24 0x00007fb7016c212c in QQmlComponentPrivate::complete (state=0x556d490c4eb8, enginePriv=0x556d490ae180) at /usr/include/x86_64-linux-gnu/qt5/QtCore/qscopedpointer.h:116
#25 QQmlComponentPrivate::complete (enginePriv=0x556d490ae180, state=0x556d490c4eb8) at qml/qqmlcomponent.cpp:996
#26 0x00007fb7016c46d9 in QQmlComponentPrivate::completeCreate (this=0x556d490c4e30) at qml/qqmlcomponent.cpp:1092
#27 0x00007fb7016c4873 in QQmlComponent::completeCreate (this=0x556d4920df10) at qml/qqmlcomponent.cpp:1079
#28 QQmlComponent::create (this=0x556d4920df10, context=<optimized out>) at qml/qqmlcomponent.cpp:825
#29 0x00007fb7017296c9 in QQmlApplicationEnginePrivate::finishLoad (this=0x556d490ae180, c=0x556d4920df10) at qml/qqmlapplicationengine.cpp:148
#30 0x00007fb701729d4c in QQmlApplicationEnginePrivate::startLoad (this=<optimized out>, url=..., data=..., dataFlag=<optimized out>) at qml/qqmlapplicationengine.cpp:132
#31 0x00007fb701729de1 in QQmlApplicationEngine::load (this=this@entry=0x7ffd007f12a0, url=...) at qml/qqmlapplicationengine.cpp:287
#32 0x0000556d47547489 in main (argc=<optimized out>, argv=<optimized out>) at ./src/main.cpp:81
[Inferior 1 (process 4097) detached]

The source code for the tool triggering the crash is available at https://invent.kde.org/system/drkonqi-pk-debug-installer/-/blob/acd7675aa72a61ede8b85fe96fc5cd225506312b/src/DebugRepoEnabler.cpp#L43 and it looks fine at least to my rusty C++ eyes.

Neon ships libappstream version 0.16.2 but a quick glance at the release log didn't look like this was fixed in the current 0.16.3 (libxmlb is at 0.3.14 already).

mss commented 1 year ago

I digged a bit and the id the tool is requesting is appstreamcli get org.kde.neon.com.ubuntu.ddebs for which appstreamcli tells me: Unable to find component with ID 'org.kde.neon.com.ubuntu.ddebs'!.

According to the code linked above does pool.componentsById(id) return at least one component nevertheless.

ximion commented 1 year ago

4 0x00007fb701fc640e in AppStream::Component::toString (this=this@entry=0x0) at ../qt/component.cpp:731

Eh... It looks like your component is nullptr and you are dereferencing NULL here. The code you linked doesn't make sense to me in what it is trying to achieve, but I also don't see how it would return a NULL component either.

    QList<AppStream::Component> components;
    for (const auto &id : ids) {
        const auto matchedComponents = pool.componentsById(id);
        components += matchedComponents;

        Q_ASSERT(components.count() == 1); // ensure distros use valid ids
        const auto &component = matchedComponents.at(0);
        Q_ASSERT(component.isValid()); // we've seen crash reports that indicated invalid components, unclear why. verify them for now.
        qWarning() << component.toString();
        qWarning() << component.packageNames();
        Q_ASSERT(component.kind() == AppStream::Component::KindRepository);
    }

=> As soon as you run through the for-loop twice and found components, components.count() will be > 1, so how does this code not crash in the respective assertion? (It would even crash in the first try when there's two repositories providing the same component...)

That's not the source of the bug though... Purely from looking at the code, I have no idea (yet) how this could happen.

ximion commented 1 year ago

I digged a bit and the id the tool is requesting is appstreamcli get org.kde.neon.com.ubuntu.ddebs for which appstreamcli tells me: Unable to find component with ID 'org.kde.neon.com.ubuntu.ddebs'!.

Also very odd - was this component maybe injected from some kind of non-stabdard location that isn't read by default by AppStream?

mss commented 1 year ago

4 0x00007fb701fc640e in AppStream::Component::toString (this=this@entry=0x0) at ../qt/component.cpp:731

Eh... It looks like your component is nullptr and you are dereferencing NULL here.

To me it looks like the component is not null but the m_cpt (or whatever valueWrap(as_component_to_string(m_cpt)) returns) is a nullptr.

The code you linked doesn't make sense to me in what it is trying to achieve [..]

Indeed, I pointed that out on the KDE bug already. I think this is a leftover of the "multiple ids aren't implemented yet" TODO further up and even though the code is slightly wrong it happens to work because it checks that only one id is given further up (ie. the whole loop is misleading).

That's not the source of the bug though... Purely from looking at the code, I have no idea (yet) how this could happen.

To me it looks like the pool.componentsById(id) returns a QList with an Component with an inner nullptr if the id can't be found. But I haven't had a chance to recompile that code with debugging enabled yet to verify this.

I digged a bit and the id the tool is requesting is appstreamcli get org.kde.neon.com.ubuntu.ddebs for which appstreamcli tells me: Unable to find component with ID 'org.kde.neon.com.ubuntu.ddebs'!.

Also very odd - was this component maybe injected from some kind of non-stabdard location that isn't read by default by AppStream?

Nope, all the code there is is in that file. The Component is provided by the Pool, nothing fancy going on.

mss commented 1 year ago

Ah, wait. I think the bug is actually in that code. This is release code so the Q_ASSERTs are probably not doing anything. So the components.count() == 1 isn't catching a zero element list being added before. And matchedComponents.at(0) would violate the requirement that the index must exist in the list, ie. it could return anything which wouldn't be caught by the next Q_ASSERT either. So the next line would be the first actual request to that invalid value.

Let me check that with the actual upstream code first, I'll get back to you and sorry for bothering you if this was not a bug in the first place.

ximion commented 1 year ago

Let me check that with the actual upstream code first, I'll get back to you and sorry for bothering you if this was not a bug in the first place.

Did your tests work? ;-)

ximion commented 1 year ago

Closing this, as it's likely not an AppStream issue. If the issue persists, feel free to reopen this issue.