Opened 6 years ago

Closed 6 years ago

#13096 closed defect (fixed)

[patch] routerlist: NULL struct pointer dereferenced to take address of element

Reported by: teor Owned by:
Priority: Medium Milestone: Tor: 0.2.5.x-final
Component: Core Tor/Tor Version: Tor: 0.2.5.5-alpha
Severity: Keywords: tor-relay
Cc: nickm Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

In routerlist.c line 4953, a struct pointer that is sometimes NULL is dereferenced by an assertion. The assertion then takes the address of one of the struct's element (routerlist.c 4953):
tor_assert(sd != &(r2->cache_info));

This is undefined behaviour in C, and could lead to the optimiser ignoring the check, or the program crashing.

To avoid dereferencing the potentially-NULL pointer, the assertion can be modified to check for NULL r2 first (if this is what is intended):

tor_assert(r2
sd != &(r2->cache_info));

The attached patch makes this change.

FYI - this error was discovered using a tor built with:
clang -fsanitize=undefined-trap -fsanitize-undefined-trap-on-error -ftrapv

Version: tor 0.2.6.?-alpha git 54348201f7cce9c0c01e9d4835714a2fec55c67c

Child Tickets

Attachments (1)

routerlist-null-pointer-deref.patch (612 bytes) - added by teor 6 years ago.
Check for NULL before dereferencing the pointer

Download all attachments as: .zip

Change History (3)

Changed 6 years ago by teor

Check for NULL before dereferencing the pointer

comment:1 Changed 6 years ago by teor

The second line of code was munged by wikiformatting:

tor_assert(!r2 || sd != &(r2->cache_info));

comment:2 Changed 6 years ago by nickm

Milestone: Tor: 0.2.5.x-final
Resolution: fixed
Status: newclosed

Committed as 3c2c6a61163cd6a42cc0eeee9fc43200b9f08503 ; thanks!

Note: See TracTickets for help on using tickets.