Hi,

I'd like to submit a pull request but, according to your "How to contribute" guide, I should submit it to the develop branch:

Once you're ready to merge your work with others, you should go to main repository and open a Pull Request to the develop branch.

However, there is no such a branch in your repository. Could you create it or can I submit the pull request to the master branch?

Thanks,

Kevin

https://github.com/Checkmarx/Go-SCP/blob/master/src/output-encoding/sql-injection.md

It could be specified that postgresql use $1, $2, $2... and that mysql use ?, ?...

customerId := r.URL.Query().Get("id")
query := "SELECT number, expireDate, cvv FROM creditcards WHERE customerId = ?"

stmt, _ := db.Query(query, customerId)
Notice the placeholder1 ? and how your query is:

On page:

https://github.com/Checkmarx/Go-SCP/blob/c3471ef24a7c2ca6a769457783f43c60712f087a/database-security/connections.md

sql.Open returns a database pool (not a connection). This can be closed just before the application exists, but databases will determine that the connections are closed when the OS terminates the network connections when the executable closes.

Your example code (while won't compile as is), shows db.QueryRow. QueryRow does not need to be manually closed; it will be closed after a Scan. Furthermore throughout I highly recommend you use the Context variants, especially with a web application. When the context is canceled, a Tx will be rolled back if not committed, a Rows (from QueryContext) will be closed, and any resources will be returned. It is important to commit/rollback a Transaction or Close a Rows either explicitly or by canceling the context. However, the example Go code does not demonstrate this.

The portion about Input Validation, and specifically sanitation of the URL request path mentions a third party package called Gorilla Toolkit, but it does not specify whether this package is also vulnerable to path traversal attacks or whether it's preferred to use because perhaps it doesn't have this vulnerability.

Can someone please advise on whether this package is also vulnerable so we can update this section to indicate that?

Hi, I am a little bit don't understand the following paragraph on the logging chapter:

... Another issue with the native logger is that there is no way to turn logging on or off on a per-package basis.

What do you mean about no way to turn logging on or off on a per-package basis. ?

As I know, you can use log.SetOutput(ioutil.Discard) to disable logging output. The example code is following:

package main

import (
	"fmt"
	"io/ioutil"
	"log"
)

func main() {
	fmt.Println("Hello, World.")
	log.SetOutput(ioutil.Discard)
	log.Print("Hello from log")
}

Or you mean this functionality does not contain inside the log package, which means you have to import io/ioutil package to achieve it ?

Thanks.

Hi,

Thanks for this piece of work. The enc/dec example here requires the nonce to be known by both methods. Is there a way to avoid passing to dec method? Similar to this. See example below - same one from the doc but split into enc and dec methods.

Thanks

package main

import (
	"crypto/aes"
	"crypto/cipher"
	"crypto/rand"
	"encoding/hex"
	"fmt"
)

func main() {
	msg := []byte("Hello World!")
	key := []byte("a6df723a921fccda52bdc4ca50851559")

	nonce := make([]byte, 12)
	if _, err := rand.Read(nonce); err != nil {
		panic(err.Error())
	}
	fmt.Println("NONCE:", hex.EncodeToString(nonce))

	enc := enc(msg, key, nonce)
	fmt.Println("ENC:", hex.EncodeToString(enc))

	dec := dec(enc, key, nonce)
	fmt.Println("DEC:", string(dec))
}

func enc(data, key, nonce []byte) []byte {
	block, err := aes.NewCipher(key)
	if err != nil {
		panic(err.Error())
	}

	aesgcm, err := cipher.NewGCM(block)
	if err != nil {
		panic(err.Error())
	}

	return aesgcm.Seal(nil, nonce, data, nil)
}

// Somehow this method should avoid requiring `nonce`.
func dec(enc, key, nonce []byte) []byte  {
	block, err := aes.NewCipher(key)
	if err != nil {
		panic(err.Error())
	}

	aesgcm, err := cipher.NewGCM(block)
	if err != nil {
		panic(err.Error())
	}

	decrypted_data, err := aesgcm.Open(nil, nonce, enc, nil)
	if err != nil {
		panic(err.Error())
	}

	return decrypted_data
}

It feels like CSRF deserves more mention in this book. There's a brief note about it in the section on websockets and the sample HTML form in "Communicating authentication data" contains a CSRF token form field, but that's it.

There's never a definition of CSRF, let alone strategies for managing or validating CSRF tokens.

On page https://github.com/Checkmarx/Go-SCP/blob/b6faece923bfdccfcc04de1b3994752721ef1ce3/docs/cryptographic-practices/README.md

both MD5 and SHA256 are discussed. SHA256 is stated to be "stronger".

It may be more precise to say that "MD5 and SHA1 may be susceptible to hash collision attacks (making the same hash from different content), while SHA256 is not known to be susceptible to such collisions."

Hi there,

This looks like a great book, but you seem to have selected the GPL license for it. I'm not sure I understand how the GPL (which is a software license) would apply to a book. Perhaps you might like to consider one of the Creative Commons licenses instead?

In the system configuration section, the sample code demonstrate disable directory listing as follows:

type justFilesFilesystem struct {
    fs http.FileSystem
}

func (fs justFilesFilesystem) Open(name string) (http.File, error) {
    f, err := fs.fs.Open(name)
    if err != nil {
        return nil, err
    }
    return neuteredReaddirFile{f}, nil
}

However, it does not provide neuteredReaddirFile type here, which only show in the files directory.

How about describe neuteredReaddirFile type here that reader can easily copy&paste the sample code when reading the content here?

If you feel ok, I can open a pull request to add it to the sample code.

Thank you.

Examples should often take extra steps to be correct. When demonstrating how to display an error message to the user: https://github.com/Checkmarx/Go-SCP/blob/c3471ef24a7c2ca6a769457783f43c60712f087a/authentication-password-management/communicating-authentication-data.md

The password check is apparently stored in plain text. I would recommend returning fields called PasswordHash and Salt then doing some fake calls to verify that against the given password in the example.

• In the Memory Management section, added a new subsection related to memory leakage scenarios.
• In Communication Security, a new subsection related to gRPC(Google Remote Procedure Call) has been added. To provide greater clarity on the topic code snippet has been added to a subdirectory under the Communication Security subdirectory.
• An entirely new section of Process Management has been added to the book. The section deals with scenarios that may result in zombie/dangling Goroutines.

The section "Sanitization" talks about what needs to be done to safely display user submitted content, which doesn't actually have anything to do with "Input Validation", despite being a part of that chapter.

Having this section in the wrong place can mislead developers and give them a false sense of security ("I don't need to worry about XSS, because I've removed the HTML stuff").

I suggest moving the "Sanitization" section to the "Output Encoding" chapter, probably renaming it to something like "HTML".

Should we remove the 1st example here in case someone doesn't read the rest of the page? https://github.com/OWASP/Go-SCP/blob/master/src/authentication-password-management/validation-and-storage.md

"sequential authentication implementations (like Google does nowadays)" in https://github.com/OWASP/Go-SCP/blob/master/src/authentication-password-management/validation-and-storage.md

In " Go-SCP/src/authentication-password-management/validation-and-storage.md " LINK the reference links for hashing functions are wrong.

It should be something similar to:

In the case of password storage, the hashing algorithms recommended by
[OWASP][2] are [`bcrypt`][3], [`PDKDF2`][4], [`Argon2`][5] and [`scrypt`][6].

[1]: ../cryptographic-practices/pseudo-random-generators.md
[2]: https://www.owasp.org/index.php/Password_Storage_Cheat_Sheet
[3]: https://godoc.org/golang.org/x/crypto/bcrypt
[4]: https://godoc.org/golang.org/x/crypto/pbkdf2
[5]: https://github.com/p-h-c/phc-winner-argon2
[6]: https://pkg.go.dev/golang.org/x/crypto/scrypt
[7]: https://github.com/ermites-io/passwd