Memory clobbering (dereferenced pointers) in event.c

[wzdev-ci]

resolution_fixed type_bug | by Buginator

---

dereferenced pointers in event.c.

More to follow in a bit.

---

Issue migrated from trac:2300 at 2022-04-16 06:45:49 -0700

[wzdev-ci]

buginator <buginator@...> commented

---

(In Warzone2100/warzone2100@1a6fb8a) Fix another memory clobbering issue. (dereferenced pointer)

Patch Author: Safety0ff
fixes #2300

Original detective work done by Ai_Tak (#1656)

Signed-off-by: buginator <buginator@>

[wzdev-ci]

Buginator changed _comment0 which not transferred by tractive

[wzdev-ci]

buginator <buginator@...> changed status from new to closed

[wzdev-ci]

buginator <buginator@...> changed resolution from `` to fixed

[wzdev-ci]

Buginator uploaded file val_no_fe.txt (1.4 KiB)

[wzdev-ci]

Buginator uploaded file val_with_fe.txt (2.0 KiB)

[wzdev-ci]

Buginator uploaded file val_wPatch_fe.txt (1.2 KiB)

[wzdev-ci]

Buginator uploaded file val_wPatch_sa_fe.txt (1.2 KiB)

[wzdev-ci]

Buginator uploaded file warzone2100.gdmp-DXGWAl.txt (9.9 KiB)

[wzdev-ci]

Buginator changed _comment0 which not transferred by tractive

[wzdev-ci]

buginator <buginator@...> commented

---

In Warzone2100/warzone2100@1a6fb8a:

#CommitTicketReference repository="" revision="1a6fb8a7547f23ce2bd7af153ac839cba999e2b0"
Fix another memory clobbering issue. (dereferenced pointer)

Patch Author: Safety0ff
fixes #2300

Original detective work done by Ai_Tak (#1656)

Signed-off-by: buginator <buginator@>

[wzdev-ci]

Buginator commented

---

The deal is, when you run normal valgrind, then you get val_no_fe.txt.
Notice the invalid reads.

When you set valgrind to fill the free()ed memory with 0xfe, (--fill-free=0xfe then you get val_with_fe.txt, and also the crash dump.

The final results are in val_wPatch_sa_fe.txt, and we don't have invalid reads anymore.

[wzdev-ci]

cybersphinx <chr.ohm@...> commented

---

(In Warzone2100/warzone2100@34112c9) Fix another memory clobbering issue. (dereferenced pointer)

Patch Author: Safety0ff
fixes #2300

Original detective work done by Ai_Tak (#1656)

Signed-off-by: buginator buginator@gna.org
(cherry picked from commit 1a6fb8a7547f23ce2bd7af153ac839cba999e2b0)

[wzdev-ci]

cybersphinx <chr.ohm@...> commented

---

In Warzone2100/warzone2100@34112c9:

#CommitTicketReference repository="" revision="34112c9e2bb5da22eac9e4a5ecb71229b6df07d2"
Fix another memory clobbering issue. (dereferenced pointer)

Patch Author: Safety0ff
fixes #2300

Original detective work done by Ai_Tak (#1656)

Signed-off-by: buginator <buginator@gna.org>
(cherry picked from commit 1a6fb8a7547f23ce2bd7af153ac839cba999e2b0)

[wzdev-ci]

Safety0ff commented

---

I don't really consider this "fixed", I've been looking at that part of the code lately and understanding it more and more.

This type of error likely only occurs when a Callback event uses the setTrigger function, but there could potentialy be other ways for setTrigger calls to mess things up.

Anyways, I'm sure that deferred deletion would solve this and it will also handle the case where multiple events are triggered and one calls setTrigger for one of those other events.

I'd rather this not be "swept under the rug".

[wzdev-ci]

buginator <buginator@...> commented

---

(In Warzone2100/warzone2100@f8517a7) Fix another memory clobbering issue. (dereferenced pointer)

Patch Author: Safety0ff
fixes #2300

Original detective work done by Ai_Tak (#1656)

Signed-off-by: buginator buginator@gna.org
(cherry picked from commit 1a6fb8a7547f23ce2bd7af153ac839cba999e2b0)

[wzdev-ci]

buginator <buginator@...> commented

---

In Warzone2100/warzone2100@f8517a7:

#CommitTicketReference repository="" revision="f8517a7716e2fe11a78fc49ca73ac92b2f7d5211"
Fix another memory clobbering issue. (dereferenced pointer)

Patch Author: Safety0ff
fixes #2300

Original detective work done by Ai_Tak (#1656)

Signed-off-by: buginator <buginator@gna.org>
(cherry picked from commit 1a6fb8a7547f23ce2bd7af153ac839cba999e2b0)

[wzdev-ci]

Safety0ff uploaded file defdelMaster.patch (10.2 KiB)

[wzdev-ci]

Safety0ff changed status from closed to reopened

[wzdev-ci]

Safety0ff changed _comment0 which not transferred by tractive

[wzdev-ci]

Safety0ff changed _comment1 which not transferred by tractive

[wzdev-ci]

Safety0ff commented

---

In the new patch, I have reverted 1a6fb8a7547f23ce2bd7af153ac839cba999e2b0 by hand and added a field to mark trigger deletion, this is a much more robust solution imho.

I also changed a function to use pointers to pointers to remove code duplication.

This patch will likely apply to the other branches as well.

I ran it with valgrind and there were no warnings.

I believe it should be possible to construct a script that causes invalid pointer dereferences under the old patch (hence the re-open).

[wzdev-ci]

Safety0ff changed resolution from fixed to ``

[wzdev-ci]

cybersphinx uploaded file defdel-cleaned-up.patch (8.6 KiB)

Rebased on a tree with 1a6fb8a7547f23ce2bd7af153ac839cba999e2b0 already reverted.

[wzdev-ci]

Safety0ff uploaded file defdelMaster.2.patch (9.5 KiB)

[wzdev-ci]

cybersphinx uploaded file defdel2-cleaned-up.patch (8.6 KiB)

Patch without the revert.

[wzdev-ci]

Safety0ff uploaded file defdelMaster.3.patch (9.0 KiB)

Fixed a bug (notice the lack of fixme)

[wzdev-ci]

Safety0ff uploaded file defdel3-cleaned-up.patch (8.4 KiB)

Same as above without revert

[wzdev-ci]

safety0ff changed status from reopened to closed

[wzdev-ci]

safety0ff changed resolution from `` to fixed

[wzdev-ci]

safety0ff commented

---

Fix an invalid pointer dereferencing issue in the script events system.

Closes #2300.
Changeset: 8c9e7d5bd0bcc0029ba50422b5b77089ed4202fe

[wzdev-ci]

safety0ff commented

---

2.3: Fix an invalid pointer dereferencing issue in the script events system.

Closes #2300.
Changeset: ddfaeca6776aac765674bb0c2a5c58ecc63a978f

[wzdev-ci]

safety0ff commented

---

3.0: Fix an invalid pointer dereferencing issue in the script events system.

Closes #2300.
Changeset: dfbceff8647ecf3e488733ed7e18e4901e4ee774

[wzdev-ci]

buginator commented

---

Fix another memory clobbering issue. (dereferenced pointer)

Patch Author: Safety0ff
fixes #2300

Original detective work done by Ai_Tak (#1656)

Signed-off-by: buginator buginator@gna.org
(cherry picked from commit 1a6fb8a7547f23ce2bd7af153ac839cba999e2b0)
Changeset: f8517a7716e2fe11a78fc49ca73ac92b2f7d5211

[wzdev-ci]

safety0ff commented

---

Fix an invalid pointer dereferencing issue in the script events system.

Closes #2300.
Changeset: 8c9e7d5bd0bcc0029ba50422b5b77089ed4202fe

[wzdev-ci]

cybersphinx changed milestone from 3.0 to unspecified

[wzdev-ci]

cybersphinx commented

---

Milestone 3.0 deleted