adamzzq / pe

0 stars 0 forks source link

Password not safe #4

Open adamzzq opened 2 months ago

adamzzq commented 2 months ago

image.png Password enable can be manually set in the txt file so that any user can enter the app without knowing the password. Moreover, the encrypted password can be manually edited (either by malicious intention of a non-user, or by the user by accident), such that the user will not be able to know the correct password.

nus-pe-script commented 2 months ago

[IMPORTANT!: Please do not edit or reply to this comment using the GitHub UI. You can respond to it using CATcher during the next phase of the PE]

Team's Response

We believe this does not constitute as a bug but rather a feature improvement that is not in scope, or a severity of low as it does not affect normal operations and does not cause any inconvenience.

As per the UG/DG description, the PIN feature is disabled by default. The fact that the password enable feature can be set manually in a text file is a design choice rather than a bug. This allows for flexibility and customization, enabling users to decide whether they want to enforce a password or not.

If the encrypted password can be edited manually, it's up to the user to understand the consequences of altering such a crucial setting. Users who make changes to sensitive settings like passwords should take responsibility for the potential impact. As per tp guidelines, this is still in line regarding advanced users being able to edit data files

The scope of this issue is limited. Only someone with access to the text file and knowledge of the system/file structure(of pin.txt in particular) would be able to manually edit the password or password enable setting. This means that an attacker would need access to the local file system to exploit the vulnerability. In a properly secured environment, this risk is already mitigated. Moreover, in the context of this course tp, the data files should be allowed to be read and be written over anyways, so the bug is trivial as any possible resolution of this will go against tp requirements. The PIN generally acts as a means to prevent possibly less advanced "malicious users" to tamper with the data via running the jar file in the local machine.

From a design perspective, allowing the user to enable or disable password protection is as a feature rather than a bug. The user has control over their security settings, allowing them to strike the right balance between ease of use and security based on their preferences.

TLDR: PIN mainly acts as a first level defence against those who tamper through using the jar file. As per the requirements of the tp, any resolution for user authentication is invalid and not within scope because data txt files should be able to write/read manually anyways.

Items for the Tester to Verify

:question: Issue response

Team chose [response.NotInScope]

Reason for disagreement: [replace this with your reason]


## :question: Issue severity Team chose [`severity.Low`] Originally [`severity.Medium`] - [ ] I disagree **Reason for disagreement:** [replace this with your reason]