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

Jump to bottom

Null pointer dereference in group window when vehicle/group dragging #8332

Closed
JGRennison opened this issue Oct 25, 2020 · 2 comments
Closed

Null pointer dereference in group window when vehicle/group dragging #8332

JGRennison opened this issue Oct 25, 2020 · 2 comments
Labels
bug Something isn't working
Milestone
1.11.0

Comments

@JGRennison
Copy link
Contributor

Version of OpenTTD

master (since 567d0ff)

Expected result

No null pointer dereferences

Actual result

A null pointer dereference

Steps to reproduce

  1. Have at least two groups and one vehicle
  2. Start dragging one of the groups
  3. Press ESC to stop dragging
  4. Delete the group
  5. Drag the vehicle onto another existing group

group_sel is set when group dragging is started, but not cleared when dragging is aborted
OnDragDrop will execute both a vehicle drag and a group drag if vehicle_sel and group_sel are both set.
OnDragDrop_Group does not check that group_sel is valid before getting the Group* and dereferencing it.
JGRennison added a commit to JGRennison/OpenTTD-patches that referenced this issue Oct 25, 2020
@JGRennison
Fix crash in group GUI vehicle/group dragging
66630ef 
See: OpenTTD/OpenTTD#8332
@James103
Copy link
Contributor

By following the above steps without deleting the group, the actual result is that not only is the vehicle assigned to the selected group, but also the last group hierarchy action that should have been aborted is instead finally completed.

Also, try the following steps:

  1. Have at least two groups and one vehicle.
  2. Start dragging one of the groups.
  3. Press ESC to stop dragging.
  4. Delete the group.
  5. In another vehicle type window, create a new group.
  6. Drag the vehicle onto another existing group of the same vehicle type.

The result is that the 'assign vehicle to group' action succeeds in step 6, but the 'set parent group' action which was initialized in step 2 persists and fails with the error message "Can't set parent group".

@TrueBrain TrueBrain added this to the 1.11.0 milestone Jan 5, 2021
TrueBrain added a commit to TrueBrain/OpenTTD that referenced this issue Jan 7, 2021
@TrueBrain
Fix OpenTTD#8332: aborting group drag&drop could cause crashes with v…
954a70a 
…ehicle drag&drop

The selected group was not reset when drag&drop was aborted. When
after that vehicle drag&drop was successful, group drag&drop code
was still executed, causing weird behaviour or even crashes.
@TrueBrain TrueBrain added the bug Something isn't working label Jan 7, 2021
@TrueBrain
Copy link
Member

Tnx for the detailed reports, and even the solution, @JGRennison ;)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug Something isn't working
Projects
None yet
Milestone
1.11.0
Development

No branches or pull requests
3 participants
@TrueBrain @JGRennison @James103