muserr / pe

0 stars 0 forks source link

JSON parsing error, did not check for integrity of file. #2

Open muserr opened 4 years ago

muserr commented 4 years ago

image.png

Issues when parsing JSON file, program did not check for integrity of file when file is modified by users who want to save time.

image.png

nus-pe-bot commented 4 years ago

Team's Response

Data files is encrypted and is saved as a .json format , thus it should not be tampered or edited by user.

A user who wants to save time would require more time to alter the save file, since they would have to break the encryption used (current implementation is multi-layered combination of AES-256 and RSA-2048) which should be highly infeasible and not alter the .json format of the file while trying to edit the data files.

Bug labelled as not in scope as "It is a valid issue but not something the team should be penalized for"

Items for the Tester to Verify

:question: Issue response

Team chose [response.NotInScope]

Reason for disagreement: I would like to thank the Developers for their response and agreement in this being a valid issue.

However, I respectfully disagree with the following:

"Data files is encrypted and is saved as a .json format , thus it should not be tampered or edited by user." There was absolutely no mention of the data files being encrypted and the way in which the data files were encrypted in the UserGuide and DeveloperGuide.

I was able only to find a mention of encryption of "user profile" in one of the developer's PPP.

image.png

(Taken from Mr Le Hong Long's PPP, pg 1)

To add onto the extract above, kindly note the following:

  1. The PPP assures users that integrity check has been performed. However, this was not properly done as clearly parsing of JSON files failed which resulted in an unhandled exception.

  2. Even more so, since "Any modification to user's stored data will be detected.", there was no proper handling of the tampered data by the program during the PE. The exception was thrown and displayed to the user instead of being handled by the program.

  3. Also, if the user data were well-encrypted, the testers will not be able to easily modify it during the one hour PE.

  4. There were no explanation or mention in the UserGuide/DeveloperGuide/PPP with regard to the multi-layer encryption: "current implementation is multi-layered combination of AES-256 and RSA-2048" as stated in the developers' response.

  5. Furthermore, it was not mentioned in the UserGuide or DeveloperGuide that users are not allowed to modify the data files.

This is a feature flaw as the developers failed to validate and take into account the proper handling and checking for integrity of data files which leads to an unhandled exception. There was also no mention of this validation to be done in V2.0.