Bug 943342 – dev-lang/R: double to long int casts overflows gc variable in memory.c

• Product: Gentoo Security
• Status: UNCONFIRMED ---

[Description gto7052 2024-11-12 22:32:54 UTC]

Hello, 

I'm not sure that this bug might really be exploitable in some malicious ways. Yet ImageMagick had a very similar bug, which was later assigned a CVE number (https://github.com/ImageMagick/ImageMagick/issues/6341) and therefore I'm posting this as a possible vulnerability here.

I was writing C-function supporting an R package and used an ASAN/UBSAN instrumented version of clang to look for potential memory holes. Instead of any hole I steadily got an "undefined behaviour" message when running a memory and gc-intense test suite manually, i.e when running all tests interactively within a single R-session, but not when R CMD check runs them file by file:

memory.c:1204:28: runtime error: 2.63525e+19 is outside the range of representable values of type 'unsigned long'
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior memory.c:1204:28

I still don't know what the actual cause is, but it certainly implies an act of negligence to cast doubles to integers and not care for an obvious overflow within a memory management routine being central to R. The relevant code section is this:

1179 static void AdjustHeapSize(R_size_t size_needed)
1180 {
1181     R_size_t R_MinNFree = (R_size_t)(orig_R_NSize * R_MinFreeFrac);
1182     R_size_t R_MinVFree = (R_size_t)(orig_R_VSize * R_MinFreeFrac);
1183     R_size_t NNeeded = R_NodesInUse + R_MinNFree;
1184     R_size_t VNeeded = R_SmallVallocSize + R_LargeVallocSize
1185         + size_needed + R_MinVFree;
1186     double node_occup = ((double) NNeeded) / R_NSize;
1187     double vect_occup = ((double) VNeeded) / R_VSize;
1188 
1189     if (node_occup > R_NGrowFrac) {
1190         R_size_t change =
1191             (R_size_t)(R_NGrowIncrMin + R_NGrowIncrFrac * R_NSize);
1192 
1193         /* for early adjustments grow more aggressively */
1194         static R_size_t last_in_use = 0;
1195         static int adjust_count = 1;
1196         if (adjust_count < 50) {
1197             adjust_count++;
1198 
1199             /* estimate next in-use count by assuming linear growth */
1200             R_size_t next_in_use = R_NodesInUse + (R_NodesInUse - last_in_use);
1201             last_in_use = R_NodesInUse;
1202 
1203             /* try to achieve and occupancy rate of R_NGrowFrac */
1204             R_size_t next_nsize = (R_size_t) (next_in_use / R_NGrowFrac);
1205             if (next_nsize > R_NSize + change)
1206                 change = next_nsize - R_NSize;
1207         }
1208 
1209         if (R_MaxNSize >= R_NSize + change)
1210             R_NSize += change;
1211     }

The double variable is R_NGrowFrac with a default value of 0.7, which can even be set from an environment variable.

A probably related effect was that the method call system of R did not find a certain method anymore.

[Comment 1 Hans de Graaff 2024-11-16 09:49:36 UTC]

Did you report this bug upstream with R? There is not much we can here from our (distribution) perspective.

[Comment 2 gto7052 2024-11-16 12:07:10 UTC]

Created attachment 908767 [details]
gdb log when breaking at AdjustHeapSize(.)

[Comment 3 gto7052 2024-11-16 12:30:25 UTC]

(In reply to Hans de Graaff from comment #1)
> Did you report this bug upstream with R? There is not much we can here from
> our (distribution) perspective.


I just lost my reply to you as I tried to attach the gdb log file.

In short: 

-The R-maintainers disallowed to create new accounts by ordinary users some time ago.  

- The reason for the overflow condition is that the assumption of a linear growth
(line 1200) does not hold, because the garbage collector may also shrink the allocated memory pool between two subsequent calls to AdjustHeapSize(.) and reflect situation that by setting R_NodesInUse to a smaller value than that one stored in the local static variable `last_in_use`. Because both variables are unsigned, this in turn may lead to a huge value for the difference `R_NodesInUse - last_in_use`, only to be noticed by UBSAN when the (implicit) integer cast from a double expression occurs in line 1204.  

As the attached debug log shows, all that leads to an absurdly wrong R_NSize of 2^63, which is the size of what R calls the "language heap".

[Comment 4 John Helmert III 2025-03-23 08:50:23 UTC]

> -The R-maintainers disallowed to create new accounts by ordinary users some time ago.  

So.. still not really much we can do from the distribution's perspective. There seems to be instructions on getting an account on their Bugzilla here, though: https://www.r-project.org/bugs.html