Repository navigation
gopher: NULL pointer dereference #3617
Description
I did this
Coverity pointed out this line (CID 1442746), as a possibly doing a NULL pointer dereference:
Line 96 in 509a0d9
It seems possible to happen if you build a URL from parts and pass to curl with CURLOPT_CURLU.
(@danielgustafsson we talked on IRC about you grabbing this, which is why you are assigned. I submitted it here just to make sure we don't forget. Feel free to unassign if you're busy.)
I expected the following
It shouldn't be necesarry to check path for NULL there. We should either bail out earlier if it is, or we should make sure that it is never NULL.
curl/libcurl version
current master (509a0d9)
operating system
All
Activity
danielgustafsson commentedon Feb 26, 2019
Thanks for adding it, I am indeed looking at this but my current travel schedule slowed things down a bit.
bagder commentedon Mar 4, 2019
I wrote up a new test to see if this
path == NULLwas actually possible to trigger, and when doing that it was obvious: no. We can't actually end up with path set to NULL in the gopher code, so the simple fix here is just to remove the check for that.When a URL is passed with
CURLOPT_URLit can't happen because then we always set a default path. The idea was then that possibly when set withCURLOPT_CURLUit would be possible. But no, due to this logic:curl/lib/transfer.c
Lines 1494 to 1502 in 2591a49
As this generates a full URL that then later is parsed to pieces again and then we get the default path if it wasn't set previously.
The new test (test 659) that verifies that this is true is part of my PR #3641 - I instead found another CURLU bug that I ended up fixing there.
gopher: remove check for path == NULL