4 BOPLA Vulns detected in the commit bee4efe

[aibot88]

Issue 1: protect.Knowledge/save

1. Risk Analysis

An attacker can inject themselves into the editors ACL during an update request, thereby bypassing authorization checks and gaining edit control over arbitrary knowledge entries. This enables unauthorized content modification and persistent privilege takeover.

---

2. Vulnerability Details

Endpoint: protect.Knowledge/save
Type: (update, knowledge.editors)

---

3. Verification Details

• Entry Point:
  KnowledgeControl.java:266 constructs KnowledgeData from request parameters.
  KnowledgeControl.java:276 performs authorization using data.getEditors().

• Issue:
  The editors field is directly sourced from the request (KnowledgeData.java:53) and used in authorization checks.

• Sink:
  KnowledgeLogic.java:254 removes existing ACL entries.
  KnowledgeLogic.java:385 rewrites KNOWLEDGE_EDIT_USERS and KNOWLEDGE_EDIT_GROUPS based on request data.

• Effective Point:
  Subsequent authorization logic continues to trust this modified ACL (KnowledgeLogic.java:1364).

• Control Comparison:
  The GET edit path retrieves editors from the database before authorization (KnowledgeControl.java:164), while the POST update path does not.

• Conclusion:
  Authorization is based on untrusted request data rather than a server-side trusted source, resulting in a broken authorization model.

---

4. Security Recommendation

Enforce authorization using trusted ACL data loaded from the database before processing updates. Only allow modification of the editors field after authorization succeeds, and never use request parameters as the basis for permission checks.

---

Issue 2: protect.Group/addUsers

1. Risk Analysis

An attacker with access to a group can add arbitrary users to that group, causing those users to inherit group-based permissions. This leads to privilege escalation, unauthorized data access, and potential content tampering.

---

2. Vulnerability Details

Endpoint: protect.Group/addUsers
Type: (addUsers, user_group.userId), (addUsers, user_group.groupRole)

---

3. Verification Details

• Entry Point:
  GroupControl.java:505 directly invokes GroupLogic.addUsers(...).

• Sink:
  GroupLogic.java:308 checks only selectAccessAbleGroup.
  GroupLogic.java:327 inserts into USER_GROUPS with role member.

• Issue:
  The “accessible group” check is weaker than the “manageable group” check (selectEditAbleGroup).

• Effective Point:
  Knowledge access and editing rely on group membership (KnowledgeLogic.java:955, KnowledgeLogic.java:1398).

• Control Comparison:
  Other methods (accept, addUsersOnEmail) enforce getEditAbleGroup() (GroupControl.java:366, GroupControl.java:539).

• Conclusion:
  The system incorrectly treats “view access” as “management permission,” allowing unauthorized group membership changes.

---

4. Security Recommendation

Restrict addUsers to users with group management privileges. Enforce server-side validation using getEditAbleGroup() or equivalent logic before inserting into USER_GROUPS.

---

Issue 3: protect.Survey/delete

1. Risk Analysis

An attacker can delete surveys and all associated data without authorization, resulting in irreversible data loss and disruption of business workflows.

---

2. Vulnerability Details

Endpoint: protect.Survey/delete
Type: (delete, survey.knowledgeId)

---

3. Verification Details

• Entry Point:
  SurveyControl.java:139 directly calls SurveyLogic.deleteSurvey(id).

• Sink:
  SurveyLogic.java:157 deletes records from:

  • SURVEYS
  • SURVEY_ITEMS
  • SURVEY_CHOICES
  • SURVEY_ANSWERS
  • SURVEY_ITEM_ANSWERS
    without any authorization checks.

• Effective Point:
  loadSurvey() (SurveyLogic.java:96) fails once data is deleted.

• Control Comparison:
  Other operations (save, report, edit, answers) enforce KnowledgeLogic.isEditor(...).

• Conclusion:
  The delete path lacks the authorization checks consistently applied in other survey operations.

---

4. Security Recommendation

Apply the same authorization checks used in other survey operations. Validate permissions using KnowledgeLogic.isEditor(...) based on the associated knowledge object before performing deletion.

---

Issue 4: File Binding in Knowledge Operations

1. Risk Analysis

An attacker can bind files uploaded by other users to a knowledge entry, causing those files to become accessible under knowledge-level permissions. This enables unauthorized file access and data exfiltration.

---

2. Vulnerability Details

Endpoints:

• protect.Knowledge/save
• protect.Knowledge/comment
• protect.Knowledge/update_comment

Type: (bind, knowledge_file.fileNo), (bind, knowledge_file.insertUser)

---

3. Verification Details

• Entry Point:
  These endpoints accept files parameters (KnowledgeControl.java:210, 266, 454, 571).

• Sink:
  UploadedFileLogic.java:151, 199 binds files when knowledgeId is null.
  KnowledgeFilesDao.java:87 performs binding without validating ownership.

• Issue:
  No validation of insertUser or draftId ownership.

• Effective Point:
  After binding, access control shifts from uploader-based to knowledge-based (UploadedFileLogic.java:320).
  Public download endpoint (FileControl.java:27) reuses this logic.

• Additional Risk:
  FILE_NO is sequential (ddl.sql:856), making enumeration feasible.

• Control Comparison:
  Unbound files enforce uploader-based access (UploadedFileLogic.java:331), but binding bypasses this restriction.

• Conclusion:
  Missing ownership validation during binding leads to unauthorized access after permission model transition.

---

4. Security Recommendation

Before binding, validate file ownership by checking insertUser or draftId against the current user/session. Only allow binding of files explicitly uploaded within the current editing context. Reject any file that fails ownership validation.