NULL pointer dereference in TRACK::GetStartNetCode()

[Erik Bosman]

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

[Erik Bosman (3y9m2-erik)]

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.

[jean-pierre charras (jp-charras)]

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