Scribunto test “LuaStandalone: SecurityTests[1]: CVE-2014-5461” failing in Wikibase CI builds
Closed, ResolvedPublic
Actions

Assigned To

Authored By

	Lucas_Werkmeister_WMDE
	Nov 28 2018, 5:22 PM

Description

In Wikidata-related repositories, we’re experiencing random CI failures with the following error (example build) since yesterday evening (I think):

There was 1 failure:

1) LuaStandalone: SecurityTests[1]: CVE-2014-5461
Failed asserting that two strings are identical.
--- Expected
+++ Actual
@@ @@
-'ERROR: stack overflow'
+'ERROR: not enough memory'

/workspace/src/extensions/Scribunto/tests/phpunit/engines/LuaCommon/LuaEngineTestBase.php:261
/workspace/src/tests/phpunit/MediaWikiTestCase.php:424
/workspace/src/maintenance/doMaintenance.php:94

This is impacting our ability to merge changes, including the fix to T210617, a train blocker.

Details

	Subject	Repo	Branch	Lines +/-
	Fix stack overflows being reported as OOM errors	mediawiki/extensions/Scribunto	master	+4 -0

Customize query in gerrit

Related Objects

Mentioned In: T209232: Add a unit test to Scribunto testing it is not vulnerable to CVE-2014-5461
Mentioned Here: rELUA7a7f5226765e: Adding a unit test for CVE-2014-5461 in Scribunto.
T209232: Add a unit test to Scribunto testing it is not vulnerable to CVE-2014-5461
T210617: BadMethodCallException on Wikidata item pages containing coordinates with non-Earth globes

Event Timeline

Lucas_Werkmeister_WMDE created this task.Nov 28 2018, 5:22 PM

Restricted Application added a subscriber: Aklapper. · View Herald TranscriptNov 28 2018, 5:22 PM

Lucas_Werkmeister_WMDE renamed this task from LuaStandalone: SecurityTests[1]: CVE-2014-5461 failing to Scribunto test “LuaStandalone: SecurityTests[1]: CVE-2014-5461” failing in Wikibase CI builds.Nov 28 2018, 5:29 PM

Lucas_Werkmeister_WMDE updated the task description. (Show Details)

Lucas_Werkmeister_WMDE added a project: Continuous-Integration-Config.

Apparently the test added for T209232: Add a unit test to Scribunto testing it is not vulnerable to CVE-2014-5461 is sometimes running out of allowed memory (set by ulimit most likely) before it runs out of Lua stack space (a constant set in Lua's C code), so it's giving a different error from the one that was expected.

If nothing else we could revert rELUA7a7f5226765e: Adding a unit test for CVE-2014-5461 in Scribunto. and let them try again on that task.

Anomie added a subscriber: Bawolff.Nov 28 2018, 7:55 PM

The gci task is already accepted - we cant force the student to do more work (we can of course ask nicely)

In T210634#4782783, @Anomie wrote:

Apparently the test added for T209232: Add a unit test to Scribunto testing it is not vulnerable to CVE-2014-5461 is sometimes running out of allowed memory (set by ulimit most likely) before it runs out of Lua stack space (a constant set in Lua's C code), so it's giving a different error from the one that was expected.

If nothing else we could revert rELUA7a7f5226765e: Adding a unit test for CVE-2014-5461 in Scribunto. and let them try again on that task.

Or you could have someone else try it again.

Also occurred on https://gerrit.wikimedia.org/r/c/mediawiki/extensions/Math/+/476339

Jdforrester-WMF subscribed.Nov 29 2018, 3:01 AM

hoo subscribed.Nov 29 2018, 3:24 AM

Michael subscribed.Nov 30 2018, 12:24 PM

Change 476854 had a related patch set uploaded (by Lucas Werkmeister (WMDE); owner: Lucas Werkmeister (WMDE)):
[mediawiki/extensions/Scribunto@master] Fix stack overflows being reported as OOM errors

https://gerrit.wikimedia.org/r/476854

gerritbot added a project: Patch-For-Review.Nov 30 2018, 1:55 PM

The change adding the test was reverted in Id2eeeb781b (I’m not sure why @gerritbot didn’t leave a comment), and since then I haven’t seen any more occurrences of this bug, so the CI issue seems to be resolved.

However, I assume we still want to have some test for CVE-2014-5461, so I’m not sure how to proceed. Perhaps close this task and reopen T209232: Add a unit test to Scribunto testing it is not vulnerable to CVE-2014-5461? (Though I’m not sure if it’s still suitable for GCI with these added complications.)

In T210634#4789618, @Lucas_Werkmeister_WMDE wrote:

The change adding the test was reverted in Id2eeeb781b (I’m not sure why @gerritbot didn’t leave a comment), and since then I haven’t seen any more occurrences of this bug, so the CI issue seems to be resolved.

@gerritbot didn't leave a comment here because Id2eeeb781b didn't mention this task in a bug footer (or maybe in the whole commit summary, I forget whether it's only the footer that counts or not).

However, I assume we still want to have some test for CVE-2014-5461, so I’m not sure how to proceed. Perhaps close this task and reopen T209232: Add a unit test to Scribunto testing it is not vulnerable to CVE-2014-5461? (Though I’m not sure if it’s still suitable for GCI with these added complications.)

Sounds good to me, I'm going to do that. Whether it's still suitable for GCI or not can be discussed on that task as well.

Anomie mentioned this in T209232: Add a unit test to Scribunto testing it is not vulnerable to CVE-2014-5461.Nov 30 2018, 6:46 PM

Aklapper removed a subscriber: Anomie.Oct 16 2020, 5:38 PM

Change 476854 abandoned by Legoktm:

[mediawiki/extensions/Scribunto@master] Fix stack overflows being reported as OOM errors

Reason:

Abandoning because the test was reverted, I think a better direction is to fix the test framework to somehow handle both errors instead of rewriting errors. (Please restore if you disagree!)