Analysis Details

Provide Sensitive Mutable Classes with Unmodifiable Wrappers

es.uniovi.reflection.analyses.cmu.obj56

We detect non-public fields that are returned by a public method, and their state may be mutated by an external client. The type of a field will be mutable by a extern client if it has a public method that may modify its state (mutators), has a mutable field which is public or returned by a public method, or has any public non-final field.Additionally, it is checked whether there exists a subclass of the field type providing an immutable wrapper to recommend using it or implementing one. In such wrapper, the mutator methods or the ones exposing the mutable fields must be overridden with implementations where the state of the object is not mutated or exposed.

Computational Reflection Research Group

12-03-2025 17:14

Security

Attribute Declaration

//Posible optimizacion, usar de terminators directamente los types que declaren terminators con sus flaws ya en un string
//de esta forma ahorramos tenerlos en la whiteList, ahorramos todos los fields/methods en la terminatorNodes ahorrando memoria
//Optimizacion de precision, mirar el tipo del field haciendo lo de getter->cfg_end->return_stmt->retExpr->its_type_is, incrementa memoria, pero si ya hemos hecho la optimización superior
//Lo de saber el tipo real de las expresiones asignadas al campo ya sería con arcos modified_by o initialized y se complica mucho, pero se podría
 MATCH ( enclosingType:TYPE_DEC {isUserCode:true, accessLevel:'public'}) - [ :DECLARES_FIELD |  INHERITS_FIELD ] ->(attr:ATTR_DEC{isStatic:false, isUserCode:true})- [ :USED_BY {isOwnAccess:true} ] -> (  :EXPRESSION ) <- [ :RETURN_EXPR ] - (:RETURN_STATEMENT )-[:CFG_NEXT]->(:CFG_NORMAL_END)-[:CFG_END_OF]->(getter:METHOD_DEC{accessLevel:'public'}),( enclosingType ) - [ getterOwnRel:DECLARES_METHOD |  INHERITS_METHOD ] -> (getter)-[:ITS_TYPE_IS]->()-[:RETURN_TYPE]->(returnType) WHERE NOT attr.accessLevel='public' 
     WITH COLLECT([attr,getter,TYPE(getterOwnRel),returnType,enclosingType]) as candidateFieldInfo //ESTOS GETTERS Y RETURN TYPES VAN AL WHITELIST, MENOS ARRAY QUE VAN AL TERMINATOR, Y VAN A STARTING LOS ATTR QUE DECLAREN EL GETTER
     
WITH [candidateInfo IN candidateFieldInfo WHERE candidateInfo[2]="DECLARES_METHOD" | [candidateInfo[0],candidateInfo[1],candidateInfo[4]]] as startingNodeInfo,
 [candidateInfo IN candidateFieldInfo WHERE candidateInfo[0].typeKind='DECLARED' | candidateInfo[1]] as whiteGetters,
  [candidateInfo IN candidateFieldInfo WHERE candidateInfo[0].typeKind='DECLARED' | candidateInfo[3]] as whiteRetTypes,
  [candidateInfo IN candidateFieldInfo WHERE candidateInfo[0].typeKind='ARRAY' | candidateInfo[1]] as termArrayGetters

WITH COLLECT(attr) as publicFinalFields, startingNodeInfo, whiteGetters, whiteRetTypes, termArrayGetters

OPTIONAL MATCH (type_def:TYPE_DEC{accessLevel:'public',isUserCode:true })

WITH COLLECT(type_def) as publicTypes, publicFinalFields,whiteGetters,whiteRetTypes, startingNodeInfo , termArrayGetters
 WITH publicTypes+publicFinalFields+whiteGetters+whiteRetTypes as whiteListNodes, startingNodeInfo , termArrayGetters
OPTIONAL MATCH ( enclosingType:TYPE_DEC {isUserCode:true, accessLevel:'public'}) -[:HAS_THIS_REFERENCE]->() - [:STATE_MODIFIED_BY |STATE_MAY_BE_MODIFIED_BY ] -> ( mutatorMethod :METHOD_DEC{accessLevel : 'public'} )

WITH COLLECT(mutatorMethod) as setters, whiteListNodes, startingNodeInfo , termArrayGetters

OPTIONAL MATCH ( enclosingType:TYPE_DEC {isUserCode:true, accessLevel:'public'}) - [ :DECLARES_FIELD |  INHERITS_FIELD ] ->(attr:ATTR_DEC{isStatic:false, isUserCode:true,accessLevel:'public'}) WHERE NOT attr.isFinal OR attr.typeKind='ARRAY'
    
WITH COLLECT(attr) as terminatorAttrs,setters,termArrayGetters,  whiteListNodes, startingNodeInfo
WITH terminatorAttrs+setters+termArrayGetters as terminatorNodes,  whiteListNodes, startingNodeInfo,termArrayGetters
UNWIND startingNodeInfo as startingNodeRow
WITH   startingNodeRow[0] as exposedField,COLLECT( startingNodeRow[1].fullyQualifiedName+'('+ startingNodeRow[1].lineNumber +')') as getters, startingNodeRow[2] as enclosingType ,termArrayGetters,terminatorNodes,  whiteListNodes

CALL apoc.path.expandConfig(exposedField, { uniqueness:'NODE_GLOBAL' ,whitelistNodes: whiteListNodes, terminatorNodes:terminatorNodes ,  relationshipFilter:'DECLARES_FIELD>|INHERITS_FIELD>|ITS_TYPE_IS>|DECLARES_METHOD>|INHERITS_METHOD>|RETURN_TYPE>', minLevel:0, optional:true, limit:1}) YIELD  path

WHERE NOT path IS NULL OR  exposedField.typeKind='ARRAY'
//GETTER WHOSE RETURNING / ATTR WHOSE / TYPE_IS TYPE IS /

WITH distinct exposedField, getters,  enclosingType, [r IN RELATIONSHIPS(path) WHERE type(r)<>'RETURN_TYPE' | CASE WHEN type(r)='ITS_TYPE_IS' THEN '' ELSE TOLOWER(REPLACE(type(r),'_',' ')) END] as relStrings

,  [n IN NODES(path) WHERE NOT n:AST_TYPE | CASE WHEN n<>LAST(NODES(path)) THEN              CASE WHEN n:METHOD_DEC THEN ' '+n.name+'(line '+n.lineNumber+'), which is public and may return an internal (non-public) field. The type returned by its method is'                                    ELSE CASE WHEN n:ATTR_DEC THEN ' '+n.name+'(line '+n.lineNumber+'), which is final, however it is exposed to the client(it is public). The type of its field is '                                        ELSE n.fullyQualifiedName+', which is public and '                                        END                                    END        ELSE            CASE WHEN n IN termArrayGetters THEN ' '+n.name+'(line '+n.lineNumber+'), which is public and thus exposes a mutable array field.' ELSE                CASE WHEN n:METHOD_DEC THEN ' '+n.name+'(line '+n.lineNumber+'), which is public and can be used to change the state of the instances.'                 ELSE CASE WHEN  NOT n.isFinal THEN ' '+n.name+'(line '+n.lineNumber+'), which is public and non-final, so it can be directly modified by a client.'                    ELSE  ' '+n.name+'(line '+n.lineNumber+'), which is public and non-final, so it can be directly modified by a client.'                    END                END            END        END]  as nodeStrings                                     , terminatorNodes,  whiteListNodes, NODES(path)[1] as fieldType

MATCH (cu:ORDINARY_CU)- [ :DECLARES_TYPE | HAS_NESTED_TYPE ] -> ( enclosingType )

WITH exposedField, getters,cu,  enclosingType , CASE WHEN fieldType IS NULL THEN 'an array type, so it can be directly modified by any client.' ELSE REDUCE( acum='',string IN [i IN RANGE(1,SIZE(nodeStrings)-2) |nodeStrings[i]+relStrings[i] ]+LAST(nodeStrings) | acum +string) END as flawDesc, fieldType    , terminatorNodes,  whiteListNodes

OPTIONAL MATCH (fieldType)< - [ :EXTENDS_CLASS | IMPLEMENTS_INTERFACE *] - ( subtype{isUserCode:true, accessLevel:'public'} ) 
   
CALL apoc.path.subgraphNodes(subtype, { whitelistNodes: whiteListNodes, terminatorNodes:terminatorNodes ,  relationshipFilter:'DECLARES_FIELD>|INHERITS_FIELD>|ITS_TYPE_IS>|DECLARES_METHOD>|INHERITS_METHOD>|RETURN_TYPE>', minLevel:0, optional:true, limit:1}) YIELD  node

WITH distinct cu, exposedField, enclosingType, getters,  [wrapperInfo IN COLLECT([subtype,node]) WHERE NOT wrapperInfo[0] IS NULL AND wrapperInfo[1] IS NULL | wrapperInfo[0].fullyQualifiedName]  as availableWrappers ,flawDesc

RETURN  DISTINCT '{"node":"FIELD '+exposedField.actualType+' '+exposedField.name+'","line":"'+exposedField.lineNumber+'","column":"'+exposedField.column+'","rule":"CMU-OBJ56","location":"'+cu.fileName+'","message":"Field ' +exposedField.name+' declared in line ' +exposedField.lineNumber+' in class '+enclosingType.fullyQualifiedName+' is not public, but it is exposed in public methods such as '+REDUCE( acum=getters[0], string IN TAIL(getters) | acum +','+string)+'. The problem is that the type of the field is '+ flawDesc +CASE WHEN SIZE(availableWrappers)=0 THEN ' You should implement an appropiate inmutable subtype as a wrapper for your attribute, as you have not created any yet.'ELSE ' Remember to use an appropiate inmutable subtype (such as '+REDUCE( acum=availableWrappers[0], string IN TAIL(availableWrappers) | acum +','+string)+') as a wrapper for your attribute.'END +'."}'