NULL pointer dereference in TRACK::GetStartNetCode()

Bug #1840562 reported by Erik Bosman
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
KiCad
Fix Released
Undecided
Unassigned

Bug Description

Affected: 5, 5.1 branch

(Suggested patch for 5.1 branch attached, not tested)

Found in 5.0.2 (debian buster):
Application: kicad
Version: 5.0.2+dfsg1-1, release build
Libraries:
    wxWidgets 3.0.4
    libcurl/7.64.0 OpenSSL/1.1.1c zlib/1.2.11 libidn2/2.0.5 libpsl/0.20.2 (+libidn2/2.0.5) libssh2/1.8.0 nghttp2/1.36.0 librtmp/2.3
Platform: Linux 4.19.0-1-amd64 x86_64, 64 bit, Little endian, wxGTK
Build Info:
    wxWidgets: 3.0.4 (wchar_t,wx containers,compatible with 2.8) GTK+ 2.24
    Boost: 1.67.0
    OpenCASCADE Community Edition: 6.9.1
    Curl: 7.62.0
    Compiler: GCC 8.2.0 with C++ ABI 1013
Build settings:
    USE_WX_GRAPHICS_CONTEXT=OFF
    USE_WX_OVERLAY=OFF
    KICAD_SCRIPTING=ON
    KICAD_SCRIPTING_MODULES=ON
    KICAD_SCRIPTING_WXPYTHON=OFF
    KICAD_SCRIPTING_ACTION_MENU=ON
    BUILD_GITHUB_PLUGIN=ON
    KICAD_USE_OCE=ON
    KICAD_USE_OCC=OFF
    KICAD_SPICE=ON

in pcbnew/editrack-part2.cpp:
TRACK::GetEndNetCode() is called as (TRACK)(NULL)->GetEndNetCode( ... ), crashes.

kicad[16425]: segfault at 50 ip 00007fa6d57da9f0 sp 00007ffde742fea8 error 4 in _pcbnew.kiface[7fa6d5055000+ab9000]

https://github.com/KiCad/kicad-source-mirror/blob/65f2af5849a4167fec9ea686a868db08ce70734e/pcbnew/editrack-part2.cpp#L295

TRACK* bufStart = m_Pcb->m_Track->GetStartNetCode( netcode ); // Beginning of tracks of the net
       ^--- can become NULL

TRACK* bufEnd = bufStart->GetEndNetCode( netcode ); // End of tracks of the net

=>

https://github.com/KiCad/kicad-source-mirror/blob/65f2af5849a4167fec9ea686a868db08ce70734e/pcbnew/class_track.cpp#L507

TRACK* TRACK::GetEndNetCode( int NetCode )
{
    TRACK* NextS, * Track = this;
    int ii = 0;

    if( Track == NULL )
        ^---- optimised out, since Track == this, and C++ probably assumes you can't use a method on a NULL pointer
        return NULL;

    if( NetCode == -1 )
        NetCode = GetNetCode();

    while( Track != NULL )
    {
        NextS = (TRACK*) Track->Pnext;
        ^----- It looks like this becomes the first thing the function does.

TRACK* TRACK::GetEndNetCode( int NetCode ):
  9c69f0: 48 8b 47 50 mov 0x50(%rdi),%rax ; comments mine: NextS = (TRACK*) Track->Pnext;
  9c69f4: 83 fe ff cmp $0xffffffff,%esi ; if( NetCode == -1 )
  9c69f7: 8b 48 4c mov 0x4c(%rax),%ecx

Tags: pcbnew
Revision history for this message
Erik Bosman (3y9m2-erik) wrote :
Revision history for this message
Erik Bosman (3y9m2-erik) wrote :

The same pointer check in TRACK::GetEndNetCode() is also present in master, but from what I can see, the method is not used at all.

Changed in kicad:
milestone: none → 5.1.5
Revision history for this message
jean-pierre charras (jp-charras) wrote :

@Eric,
Your patch looks good to me. I committed it.
Thanks.

Changed in kicad:
status: New → Fix Committed
tags: added: pcbnew
Changed in kicad:
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.