Skip to content

Commit

Permalink
lib/ogsf: fix possible overflow errors in gsd_surf.c (#4635)
Browse files Browse the repository at this point in the history 
ogsf: fix possible overflow errors in gsd modules

In a lot of places, `(255 << 24)` which causes integer overflow
and positive number gets converted to negative number. We
were then assigning this to an unsigned integer in multiple
places, which does conversion in a different way.

For example: If we do unsigned int x = -20, `UINT_MAX + 1 - 20`
is assigned to x.

I do not think that's what is intended when we do
`ktrans = (255 << 24)`. Fix instances of that, by using an
unsigned int literal over int literal.

This issue was found using cppcheck tool.

Signed-off-by: Mohan Yelugoti <[email protected]>
  • Loading branch information
@ymdatta
ymdatta authored Nov 5, 2024
1 parent 16c5f6f commit f7537c4
Showing 1 changed file with 9 additions and 9 deletions.
18 changes: 9 additions & 9 deletions lib/ogsf/gsd_surf.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -228,7 +228,7 @@ int gsd_surf_map_old(geosurf *surf)
*/
check_transp = 0;
tratt = &(surf->att[ATT_TRANSP]);
ktrans = (255 << 24);
ktrans = (255U << 24);
trans_src = surf->att[ATT_TRANSP].att_src;

if (CONST_ATT == trans_src && surf->att[ATT_TRANSP].constant != 0.0) {
Expand Down Expand Up @@ -344,7 +344,7 @@ int gsd_surf_map_old(geosurf *surf)
if (check_transp) {
GET_MAPATT(trbuff, offset, ttr);
ktrans = (char)SCALE_ATT(tratt, ttr, 0, 255);
ktrans = (char)(255 - ktrans) << 24;
ktrans = (char)(255U - ktrans) << 24;
}

gsd_litvert_func(n, ktrans | curcolor, pt);
Expand All @@ -369,7 +369,7 @@ int gsd_surf_map_old(geosurf *surf)
if (check_transp) {
GET_MAPATT(trbuff, offset, ttr);
ktrans = (char)SCALE_ATT(tratt, ttr, 0, 255);
ktrans = (char)(255 - ktrans) << 24;
ktrans = (char)(255U - ktrans) << 24;
}

if (check_material) {
Expand Down Expand Up @@ -469,7 +469,7 @@ int gsd_surf_map_old(geosurf *surf)
if (check_transp) {
GET_MAPATT(trbuff, offset, ttr);
ktrans = (char)SCALE_ATT(tratt, ttr, 0, 255);
ktrans = (char)(255 - ktrans) << 24;
ktrans = (char)(255U - ktrans) << 24;
}

if (check_material) {
Expand Down Expand Up @@ -524,7 +524,7 @@ int gsd_surf_map_old(geosurf *surf)
if (check_transp) {
GET_MAPATT(trbuff, offset, ttr);
ktrans = (char)SCALE_ATT(tratt, ttr, 0, 255);
ktrans = (char)(255 - ktrans) << 24;
ktrans = (char)(255U - ktrans) << 24;
}

if (check_material) {
Expand Down Expand Up @@ -580,7 +580,7 @@ int gsd_surf_map_old(geosurf *surf)
if (check_transp) {
GET_MAPATT(trbuff, offset, ttr);
ktrans = (char)SCALE_ATT(tratt, ttr, 0, 255);
ktrans = (char)(255 - ktrans) << 24;
ktrans = (char)(255U - ktrans) << 24;
}

if (check_material) {
Expand Down Expand Up @@ -649,7 +649,7 @@ int gsd_surf_map_old(geosurf *surf)
if (check_transp) {
GET_MAPATT(trbuff, offset, ttr);
ktrans = (char)SCALE_ATT(tratt, ttr, 0, 255);
ktrans = (char)(255 - ktrans) << 24;
ktrans = (char)(255U - ktrans) << 24;
}

if (check_material) {
Expand Down Expand Up @@ -2144,7 +2144,7 @@ int gsd_surf_map(geosurf *surf)
*/
check_transp = 0;
tratt = &(surf->att[ATT_TRANSP]);
ktrans = (255 << 24);
ktrans = (255U << 24);
trans_src = surf->att[ATT_TRANSP].att_src;

if (CONST_ATT == trans_src && surf->att[ATT_TRANSP].constant != 0.0) {
Expand Down Expand Up @@ -2328,7 +2328,7 @@ int gsd_surf_map(geosurf *surf)
if (check_transp) {
GET_MAPATT(trbuff, offset2[ii], ttr);
ktrans = (char)SCALE_ATT(tratt, ttr, 0, 255);
ktrans = (char)(255 - ktrans) << 24;
ktrans = (char)(255U - ktrans) << 24;
}

if (check_material) {
Expand Down

0 comments on commit f7537c4

Please sign in to comment.