Skip to content

Commit 5feaca4

Browse files
committed
Fix some checks if framebuffer is active
As potential security vulnerability to shutdown the process, it is required to check if the framebuffer is really in use. Without the check, a Python user could shutdown the program by trying to execute a draw function on a not opened framebuffer without initialized offscreen buffer. The result would be a NULL pointer access to the offscreen buffer. Additional to closing this security vulnerability, adds a function to check if a framebuffer is opened only for internal purpose in the validation process before executing the draw operation.
1 parent 142bd1c commit 5feaca4

File tree

2 files changed

+53
-0
lines changed

2 files changed

+53
-0
lines changed

native/framebuffers.c

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,15 @@ void pyfb_init(void) {
2626
}
2727
}
2828

29+
int __APISTATUS_internal pyfb_fbused(uint8_t fbnum) {
30+
if(framebuffers[fbnum].fb_fd == -1) {
31+
return 0;
32+
}
33+
34+
// else
35+
return 1;
36+
}
37+
2938
void __APISTATUS_internal pyfb_fblock(uint8_t fbnum) {
3039
// first test if this device number is valid
3140
if(fbnum >= MAX_FRAMEBUFFERS) {
@@ -315,6 +324,13 @@ void pyfb_ssetPixel(uint8_t fbnum, unsigned long int x, unsigned long int y, con
315324
// Is valid, so lock it!
316325
lock(framebuffers[fbnum].fb_lock);
317326

327+
// next, test if the device is really in use
328+
if(framebuffers[fbnum].fb_fd == -1) {
329+
// this framebuffer is not in use, so ignore
330+
unlock(framebuffers[fbnum].fb_lock);
331+
return;
332+
}
333+
318334
// check if all values are valid
319335
unsigned int xres = framebuffers[fbnum].fb_info.vinfo.xres;
320336
unsigned int yres = framebuffers[fbnum].fb_info.vinfo.yres;
@@ -362,6 +378,13 @@ void pyfb_sdrawHorizontalLine(uint8_t fbnum,
362378
// Is valid, so lock it!
363379
lock(framebuffers[fbnum].fb_lock);
364380

381+
// next, test if the device is really in use
382+
if(framebuffers[fbnum].fb_fd == -1) {
383+
// this framebuffer is not in use, so ignore
384+
unlock(framebuffers[fbnum].fb_lock);
385+
return;
386+
}
387+
365388
// Now test if the ranges are okay
366389
unsigned long int xres = framebuffers[fbnum].fb_info.vinfo.xres;
367390
unsigned long int yres = framebuffers[fbnum].fb_info.vinfo.yres;
@@ -407,6 +430,13 @@ void pyfb_sdrawVerticalLine(uint8_t fbnum,
407430
// Is valid, so lock it!
408431
lock(framebuffers[fbnum].fb_lock);
409432

433+
// next, test if the device is really in use
434+
if(framebuffers[fbnum].fb_fd == -1) {
435+
// this framebuffer is not in use, so ignore
436+
unlock(framebuffers[fbnum].fb_lock);
437+
return;
438+
}
439+
410440
// Now test if the ranges are okay
411441
unsigned long int xres = framebuffers[fbnum].fb_info.vinfo.xres;
412442
unsigned long int yres = framebuffers[fbnum].fb_info.vinfo.yres;

native/pyframebuffer.h

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -250,6 +250,29 @@ extern void __APISTATUS_internal pyfb_fblock(uint8_t fbnum);
250250
*/
251251
extern void __APISTATUS_internal pyfb_fbunlock(uint8_t fbnum);
252252

253+
/**
254+
* Checks if the framebuffer is really opened. This is only used internally as checking function before
255+
* executing a draw operation to prevent NULL pointer access.
256+
*
257+
* The access should work like this:
258+
* \code{.c}
259+
* if(!pyfb_fbused(fbnum)) {
260+
* // any error handling
261+
* return;
262+
* }
263+
*
264+
* // if get here, the framebuffer is in use
265+
* \endcode
266+
*
267+
* Please lock the framebuffer before invoking this function. This ensures that the status request is keept
268+
* up to date or is not modified at the same time this function is testing it.
269+
*
270+
* @param fbnum The framebuffer number to check
271+
*
272+
* @return If is in use 1, else 0
273+
*/
274+
extern int __APISTATUS_internal pyfb_fbused(uint8_t fbnum);
275+
253276
/**
254277
* Paints a exactly horizontal line. This function is secure because before painting,
255278
* it validates the arguments.

0 commit comments

Comments
 (0)