From 5feaca4ca96da752a6d3d9b061a76bb202825f0f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adrian=20Ro=C3=9F?= Date: Sat, 27 Jul 2024 17:57:09 +0000 Subject: [PATCH] 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. --- native/framebuffers.c | 30 ++++++++++++++++++++++++++++++ native/pyframebuffer.h | 23 +++++++++++++++++++++++ 2 files changed, 53 insertions(+) diff --git a/native/framebuffers.c b/native/framebuffers.c index 3b5d8de..e3730d7 100644 --- a/native/framebuffers.c +++ b/native/framebuffers.c @@ -26,6 +26,15 @@ void pyfb_init(void) { } } +int __APISTATUS_internal pyfb_fbused(uint8_t fbnum) { + if(framebuffers[fbnum].fb_fd == -1) { + return 0; + } + + // else + return 1; +} + void __APISTATUS_internal pyfb_fblock(uint8_t fbnum) { // first test if this device number is valid if(fbnum >= MAX_FRAMEBUFFERS) { @@ -315,6 +324,13 @@ void pyfb_ssetPixel(uint8_t fbnum, unsigned long int x, unsigned long int y, con // Is valid, so lock it! lock(framebuffers[fbnum].fb_lock); + // next, test if the device is really in use + if(framebuffers[fbnum].fb_fd == -1) { + // this framebuffer is not in use, so ignore + unlock(framebuffers[fbnum].fb_lock); + return; + } + // check if all values are valid unsigned int xres = framebuffers[fbnum].fb_info.vinfo.xres; unsigned int yres = framebuffers[fbnum].fb_info.vinfo.yres; @@ -362,6 +378,13 @@ void pyfb_sdrawHorizontalLine(uint8_t fbnum, // Is valid, so lock it! lock(framebuffers[fbnum].fb_lock); + // next, test if the device is really in use + if(framebuffers[fbnum].fb_fd == -1) { + // this framebuffer is not in use, so ignore + unlock(framebuffers[fbnum].fb_lock); + return; + } + // Now test if the ranges are okay unsigned long int xres = framebuffers[fbnum].fb_info.vinfo.xres; unsigned long int yres = framebuffers[fbnum].fb_info.vinfo.yres; @@ -407,6 +430,13 @@ void pyfb_sdrawVerticalLine(uint8_t fbnum, // Is valid, so lock it! lock(framebuffers[fbnum].fb_lock); + // next, test if the device is really in use + if(framebuffers[fbnum].fb_fd == -1) { + // this framebuffer is not in use, so ignore + unlock(framebuffers[fbnum].fb_lock); + return; + } + // Now test if the ranges are okay unsigned long int xres = framebuffers[fbnum].fb_info.vinfo.xres; unsigned long int yres = framebuffers[fbnum].fb_info.vinfo.yres; diff --git a/native/pyframebuffer.h b/native/pyframebuffer.h index bd969a5..70912e9 100644 --- a/native/pyframebuffer.h +++ b/native/pyframebuffer.h @@ -250,6 +250,29 @@ extern void __APISTATUS_internal pyfb_fblock(uint8_t fbnum); */ extern void __APISTATUS_internal pyfb_fbunlock(uint8_t fbnum); +/** + * Checks if the framebuffer is really opened. This is only used internally as checking function before + * executing a draw operation to prevent NULL pointer access. + * + * The access should work like this: + * \code{.c} + * if(!pyfb_fbused(fbnum)) { + * // any error handling + * return; + * } + * + * // if get here, the framebuffer is in use + * \endcode + * + * Please lock the framebuffer before invoking this function. This ensures that the status request is keept + * up to date or is not modified at the same time this function is testing it. + * + * @param fbnum The framebuffer number to check + * + * @return If is in use 1, else 0 + */ +extern int __APISTATUS_internal pyfb_fbused(uint8_t fbnum); + /** * Paints a exactly horizontal line. This function is secure because before painting, * it validates the arguments.