On Sat, Feb 16, 2019 at 10:27:30AM +0100, Theo Buehler wrote:
> When running R with vm.malloc_conf=F, doing
>
> > install.package("any")
>
> on a fresh R install leads a segfault due to a use-after-free in
> src/modules/internet/libcurl.c:
>
> 644 for (int i = 0; i < nurls; i++) {
>
> ...
>
> here hnd[0] is freed:
>
> 656 curl_easy_cleanup(hnd[i]);
> 657 }
> 658 // This can show an invalid read: can it be improved?
> 659 long status = 0L;
> 660 if(nurls == 1)
> 661 curl_easy_getinfo(hnd[0], CURLINFO_RESPONSE_CODE, &status);
>
> and here it is used again to retrieve the status code.
>
> Note that the value of status is used later only in case nurls == 1
> for error reporting. The patch below should therefore preserve the
> intended behavior since curl_easy_getinfo() is idempotent.
OK feinerer@
Upstream commit has SVN revision 76143
(https://github.com/wch/r-source/commit/36bcd78ffd71547e1132903c00c5a6dae6e43609)
> Index: Makefile
> ===================================================================
> RCS file: /var/cvs/ports/math/R/Makefile,v
> retrieving revision 1.105
> diff -u -p -r1.105 Makefile
> --- Makefile 23 Dec 2018 08:03:45 -0000 1.105
> +++ Makefile 16 Feb 2019 08:54:02 -0000
> @@ -2,6 +2,7 @@
>
> COMMENT= powerful math/statistics/graphics language
> DISTNAME= R-3.5.2
> +REVISION= 0
>
> SO_VERSION= 34.2
> .for _lib in R Rblas Rlapack
> Index: patches/patch-src_modules_internet_libcurl_c
> ===================================================================
> RCS file: patches/patch-src_modules_internet_libcurl_c
> diff -N patches/patch-src_modules_internet_libcurl_c
> --- /dev/null 1 Jan 1970 00:00:00 -0000
> +++ patches/patch-src_modules_internet_libcurl_c 16 Feb 2019 08:43:53
> -0000
> @@ -0,0 +1,31 @@
> +$OpenBSD$
> +
> +Avoid use-after-free.
> +Index: src/modules/internet/libcurl.c
> +--- src/modules/internet/libcurl.c.orig
> ++++ src/modules/internet/libcurl.c
> +@@ -641,12 +641,12 @@ in_do_curlDownload(SEXP call, SEXP op, SEXP args, SEXP
> +
> + n_err += curlMultiCheckerrs(mhnd);
> +
> ++ long status = 0L;
> + for (int i = 0; i < nurls; i++) {
> + if (out[i]) {
> + fclose(out[i]);
> + double dl;
> + curl_easy_getinfo(hnd[i], CURLINFO_SIZE_DOWNLOAD, &dl);
> +- long status;
> + curl_easy_getinfo(hnd[i], CURLINFO_RESPONSE_CODE, &status);
> + // should we do something about incomplete transfers?
> + if (status != 200 && dl == 0. && strchr(mode, 'w'))
> +@@ -655,10 +655,6 @@ in_do_curlDownload(SEXP call, SEXP op, SEXP args, SEXP
> + curl_multi_remove_handle(mhnd, hnd[i]);
> + curl_easy_cleanup(hnd[i]);
> + }
> +- // This can show an invalid read: can it be improved?
> +- long status = 0L;
> +- if(nurls == 1)
> +- curl_easy_getinfo(hnd[0], CURLINFO_RESPONSE_CODE, &status);
> + curl_multi_cleanup(mhnd);
> + if (!cacheOK) curl_slist_free_all(slist1);
> +
