Skip to content

Commit f12d6e9

Browse files
aranhamsGabhenDM
authored andcommitted
suggested modifications in the comments
1 parent ecb36ba commit f12d6e9

File tree

6 files changed

+65
-51
lines changed

6 files changed

+65
-51
lines changed

owasp-top10-2021-apps/a1/camplake-api/README.md

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,7 @@ curl -s -H "Content-Type: application/json" -d '{"username":"campLakeAdmin","pas
6666
With the created user, we will login to the application with their credentials to get the JWT token. As it is a test application, the JWT token is returned to the user as soon as he is logged in.
6767

6868
```sh
69-
curl -s -H "Content-Type: application/json" -d '{"username":"campLakeAdmin","password":"campLake2021"}' http://localhost:10007/login
69+
curl -s -H "Content-Type: application/json" -d '{"username":"campLakeAdmin","password":"campLake2021"}' http://localhost:20001/login
7070
```
7171

7272
<p align="center">
@@ -92,22 +92,24 @@ curl -s -H 'Content-Type: application/json' -H 'Authorization: Bearer eyJhbGciOi
9292
However, the API does not verify the signature used by the JWT token, any malicious user can create a fake token, as shown by the image:
9393

9494
<p align="center">
95-
<img src="images/attack_6.png"/>
95+
<img src="images/attack_5.png"/>
9696
</p>
9797

9898
```sh
9999
curl -s -H 'Content-Type: application/json' -H 'Authorization: Bearer eyJhbGciOiJub25lIiwidHlwIjoiSldUIn0.eyJ1c2VybmFtZSI6Imphc29uVm9vcmhlc3MiLCJleHAiOjE2MzMzODM1ODZ9.' -d '{"title": "New member ", "post": "Today a new member ..."}' http://localhost:20001/newpost
100100
```
101101

102102
<p align="center">
103-
<img src="images/attack_5.png"/>
103+
<img src="images/attack_6.png"/>
104104
</p>
105105

106+
106107
## Secure this app
107108

108109
How would you mitigate this vulnerability? After your changes, an attacker should not be able to:
109110

110-
* Check and validate JWT Token signature.
111+
* Use fake tokens without a valid signature.
112+
* Impersonate other users through manipulation of the JWT.
111113

112114
## PR solutions
113115

owasp-top10-2021-apps/a1/camplake-api/app/go.mod

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ require (
77
github.com/google/uuid v1.3.0
88
github.com/labstack/echo v3.3.10+incompatible
99
github.com/labstack/gommon v0.3.0 // indirect
10+
github.com/sirupsen/logrus v1.8.1
1011
github.com/stretchr/testify v1.7.0 // indirect
1112
golang.org/x/crypto v0.0.0-20210921155107-089bfa567519
1213
gopkg.in/mgo.v2 v2.0.0-20190816093944-a6b53ec6cb22

owasp-top10-2021-apps/a1/camplake-api/app/go.sum

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
1-
github.com/davecgh/go-spew v1.1.0 h1:ZDRjVQ15GmhC3fiQ8ni8+OwkZQO4DARzQgrnXU1Liz8=
21
github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
2+
github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c=
3+
github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
34
github.com/dgrijalva/jwt-go v3.2.0+incompatible h1:7qlOGliEKZXTDg6OTjfoBKDXWrumCAMpl/TFQ4/5kLM=
45
github.com/dgrijalva/jwt-go v3.2.0+incompatible/go.mod h1:E3ru+11k8xSBh+hMPgOLZmtrrCbhqsmaPHjLKYnJCaQ=
56
github.com/google/uuid v1.3.0 h1:t6JiXgmwXMjEs8VusXIJk2BXHsn+wx8BZdTaoZ5fu7I=
@@ -15,7 +16,10 @@ github.com/mattn/go-isatty v0.0.9 h1:d5US/mDsogSGW37IV293h//ZFaeajb69h+EHFsv2xGg
1516
github.com/mattn/go-isatty v0.0.9/go.mod h1:YNRxwqDuOph6SZLI9vUUz6OYw3QyUt7WiY2yME+cCiQ=
1617
github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM=
1718
github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4=
19+
github.com/sirupsen/logrus v1.8.1 h1:dJKuHgqk1NNQlqoA6BTlM1Wf9DOH3NBjQyu0h9+AZZE=
20+
github.com/sirupsen/logrus v1.8.1/go.mod h1:yWOB1SBYBC5VeMP7gHvWumXLIWorT60ONWic61uBYv0=
1821
github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME=
22+
github.com/stretchr/testify v1.2.2/go.mod h1:a8OnRcib4nhh0OaRAV+Yts87kKdq0PP7pXfy6kDkUVs=
1923
github.com/stretchr/testify v1.4.0/go.mod h1:j7eGeouHqKxXV5pUuKE4zz7dFj8WfuZ+81PSLYec5m4=
2024
github.com/stretchr/testify v1.7.0 h1:nwc3DEeHmmLAfoZucVR881uASk0Mfjw8xYJ99tb5CcY=
2125
github.com/stretchr/testify v1.7.0/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg=
@@ -29,6 +33,7 @@ golang.org/x/net v0.0.0-20210226172049-e18ecbb05110 h1:qWPm9rbaAMKs8Bq/9LRpbMqxW
2933
golang.org/x/net v0.0.0-20210226172049-e18ecbb05110/go.mod h1:m0MpNAwzfU5UDzcl9v0D8zg8gWTRqZa9RBIspLL5mdg=
3034
golang.org/x/sys v0.0.0-20190222072716-a9d3bda3a223/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY=
3135
golang.org/x/sys v0.0.0-20190813064441-fde4db37ae7a/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
36+
golang.org/x/sys v0.0.0-20191026070338-33540a1f6037/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
3237
golang.org/x/sys v0.0.0-20201119102817-f84b799fce68/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
3338
golang.org/x/sys v0.0.0-20210615035016-665e8c7367d1 h1:SrN+KX8Art/Sf4HNj6Zcz06G7VEz+7w9tdXTPOZ7+l4=
3439
golang.org/x/sys v0.0.0-20210615035016-665e8c7367d1/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=

owasp-top10-2021-apps/a1/camplake-api/app/handlers/handlers.go

Lines changed: 52 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import (
1010

1111
"github.com/google/uuid"
1212
"github.com/labstack/echo"
13+
log "github.com/sirupsen/logrus"
1314
)
1415

1516
func HealthCheck(c echo.Context) error {
@@ -27,6 +28,11 @@ func WriteCookie(c echo.Context, jwt string) error {
2728
func ReadCookie(c echo.Context) error {
2829
cookie, err := c.Cookie("sessionIDa5")
2930
if err != nil {
31+
log.WithFields(
32+
log.Fields{
33+
"method": "ReadCookie",
34+
"error": err,
35+
}).Error()
3036
return err
3137
}
3238
fmt.Println(cookie.Name)
@@ -38,17 +44,26 @@ func NewUser(c echo.Context) error {
3844
userData := types.UserData{}
3945
err := c.Bind(&userData)
4046
if err != nil {
41-
return c.JSON(http.StatusBadRequest, map[string]string{"result": "error", "details": "Error user data1."})
47+
log.WithFields(
48+
log.Fields{
49+
"method": "NewUserData",
50+
"error": err,
51+
}).Error()
52+
return c.JSON(http.StatusBadRequest, map[string]string{"result": "error", "details": "Error user data."})
4253
}
4354

4455
newGUID := uuid.Must(uuid.NewRandom())
4556
userData.UserID = newGUID.String()
4657

4758
err = db.RegisterUser(userData)
4859
if err != nil {
60+
log.WithFields(
61+
log.Fields{
62+
"method": "NewUserRegisterUser",
63+
"error": err,
64+
}).Error()
4965
errorString := fmt.Sprintf("%s", err)
5066
return c.JSON(http.StatusBadRequest, map[string]string{"result": "error", "details": errorString})
51-
5267
}
5368

5469
return c.String(http.StatusCreated, "Register: success!\n")
@@ -58,22 +73,37 @@ func Login(c echo.Context) error {
5873
loginAttempt := types.LoginAttempt{}
5974
err := c.Bind(&loginAttempt)
6075
if err != nil {
61-
return c.JSON(http.StatusBadRequest, map[string]string{"result": "error", "details": "Error login1."})
76+
log.WithFields(
77+
log.Fields{
78+
"method": "LoginLoginAttempt",
79+
"error": err,
80+
}).Error()
81+
return c.JSON(http.StatusBadRequest, map[string]string{"result": "error", "details": "Incorrect username or password."})
6282
}
6383

6484
userDataQuery := map[string]interface{}{"username": loginAttempt.Username}
6585
userDataResult, err := db.GetUserData(userDataQuery)
6686
if err != nil {
67-
return c.JSON(http.StatusBadRequest, map[string]string{"result": "error", "details": "Error login3."})
87+
log.WithFields(
88+
log.Fields{
89+
"method": "LoginUserDataQuery",
90+
"error": err,
91+
}).Error()
92+
return c.JSON(http.StatusBadRequest, map[string]string{"result": "error", "details": "Incorrect username or password."})
6893
}
6994

7095
passOK := crypto.CheckPasswordHash(loginAttempt.Password, userDataResult.HashedPassword)
7196
if err != nil {
72-
return c.JSON(http.StatusBadRequest, map[string]string{"result": "error", "details": "Error login2."})
97+
log.WithFields(
98+
log.Fields{
99+
"method": "LoginCheckPasswordHash",
100+
"error": err,
101+
}).Error()
102+
return c.JSON(http.StatusBadRequest, map[string]string{"result": "error", "details": "Incorrect username or password."})
73103
}
74104

75105
if !passOK {
76-
return c.JSON(http.StatusBadRequest, map[string]string{"result": "error", "details": "Error login4."})
106+
return c.JSON(http.StatusBadRequest, map[string]string{"result": "error", "details": "Incorrect username or password."})
77107
}
78108

79109
creds := types.Credentials{
@@ -82,12 +112,22 @@ func Login(c echo.Context) error {
82112
// Create token
83113
token, err := CreateToken(creds)
84114
if err != nil {
115+
log.WithFields(
116+
log.Fields{
117+
"method": "LoginCreateToken",
118+
"error": err,
119+
}).Error()
85120
return err
86121
}
87122

88123
err = WriteCookie(c, token)
89124
if err != nil {
90-
return c.JSON(http.StatusBadRequest, map[string]string{"result": "error", "details": "Error login5."})
125+
log.WithFields(
126+
log.Fields{
127+
"method": "LoginWriteCookie",
128+
"error": err,
129+
}).Error()
130+
return c.JSON(http.StatusBadRequest, map[string]string{"result": "error", "details": "Incorrect username or password"})
91131
}
92132

93133
messageLogon := fmt.Sprintf("Hello, %s! This is your token: %s\n", userDataResult.Username, token)
@@ -97,6 +137,11 @@ func Login(c echo.Context) error {
97137
func NewPost(c echo.Context) error {
98138
var td *types.Post
99139
if err := c.Bind(&td); err != nil {
140+
log.WithFields(
141+
log.Fields{
142+
"method": "NewPost",
143+
"error": err,
144+
}).Error()
100145
return c.JSON(http.StatusUnprocessableEntity, "invalid json")
101146
}
102147

owasp-top10-2021-apps/a1/camplake-api/app/handlers/jwt.go

Lines changed: 0 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -41,41 +41,6 @@ func ExtractToken(r *http.Request) string {
4141
return ""
4242
}
4343

44-
// func TokenValid(r *http.Request) error {
45-
// token := ExtractToken(r)
46-
// t := strings.Split(token, ".")
47-
48-
// header := types.Header{}
49-
// claims := types.Claims{}
50-
51-
// h, err := base64.StdEncoding.WithPadding(base64.NoPadding).DecodeString(t[0])
52-
// if err != nil {
53-
// return err
54-
// }
55-
56-
// err = json.Unmarshal(h, &header)
57-
// if err != nil {
58-
// log.Fatalln("Error in JSON unmarshalling from json marshalled object:", err)
59-
// return err
60-
// }
61-
// if header.Typ != "JWT" {
62-
// log.Fatalln("Error on JWT")
63-
// }
64-
65-
// c, err := base64.StdEncoding.WithPadding(base64.NoPadding).DecodeString(t[1])
66-
// if err != nil {
67-
// return err
68-
// }
69-
70-
// err = json.Unmarshal(c, &claims)
71-
// if err != nil {
72-
// log.Fatalln("Error in JSON unmarshalling from json marshalled object:", err)
73-
// return err
74-
// }
75-
76-
// return nil
77-
// }
78-
7944
func TokenValid(r *http.Request) (types.Claims, error) {
8045
header := types.Header{}
8146
claims := types.Claims{}

owasp-top10-2021-apps/a1/camplake-api/app/server.go

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -95,10 +95,6 @@ func main() {
9595
echoInstance := echo.New()
9696
echoInstance.HideBanner = true
9797

98-
// echoInstance.Use(middleware.Logger())
99-
// echoInstance.Use(middleware.Recover())
100-
// echoInstance.Use(middleware.RequestID())
101-
10298
fakeUser, _ := CreateFakeToken()
10399
log.Printf("Fake Token %s", fakeUser)
104100

0 commit comments

Comments
 (0)