oscarngncc / COMP4111_project

COMP4111 project
0 stars 0 forks source link

Illegal requests with invalid paths successfully update the database #23

Open STommydx opened 4 years ago

STommydx commented 4 years ago

Describe the bug Requests with invalid paths successfully update the database. To be particular, the subpaths between books and id (exclusive) are ignored.

To Reproduce

  1. Login as usual
  2. Add a book. The book id should be 1 for first run.
    {
    "Title": "Delete me yo!",
    "Author": "Bug Hunter",
    "Publisher": "Bug Bounty Inc.",
    "Year": 2020
    }
  3. Send an illegal request as below DELETE /BookManagementService/books/hi/1?token={{token}}

Expected behavior 400 Bad Request or 404 Not Found or other reasonable custom behavior The book remains in the database, which can be verified with a lookup quest. GET /BookManagementService/books?id=1&token={{token}}

Response of lookup request: 200 OK

{
    "FoundBooks": 1,
    "Results": [
        {
            "Title": "Delete me yo!",
            "Author": "Bug Hunter",
            "Publisher": "Bug Bounty Inc.",
            "Year": 2020
        }
    ]
}

What actually happens 200 OK The book is indeed deleted, which can be verified with a lookup quest. GET /BookManagementService/books?id=1&token={{token}}

Response of lookup request: 204 No content

oscarngncc commented 4 years ago

Similar issue actually happened in the first bounty program before when I try to use a wrong URL request to add and get a book. We even have some argument here about exception caused by using a wrong URL request should be a bug or not:

https://github.com/chihimng/COMP4111_Project/issues/12 https://github.com/chihimng/COMP4111_Project/issues/11

However, according to TA, this is not considered as a bug. (Undefined bug by spec) image

STommydx commented 4 years ago

I would personally consider this as a security issue more. It is an unintended access and modification of the records. @comp4111ta has clarified in the "End of phase 1" email: any security problem with real impacts are bugs. Let's leave the decision to @comp4111ta.

Anyway, many groups (including myself) has this same issue. You are not alone lol.

comp4111ta commented 4 years ago

Hi, this is undefined behavior. If you can get the data from the database, it is a security problem.