Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Potential file inclusion (CWE-22) & Deferring unsafe Method (CWE-703) #223

Open
0x9a opened this issue Mar 2, 2022 · 0 comments
Open

Potential file inclusion (CWE-22) & Deferring unsafe Method (CWE-703) #223

0x9a opened this issue Mar 2, 2022 · 0 comments

Comments

@0x9a
Copy link

0x9a commented Mar 2, 2022

Hi Sir I Have 2 bug in your programming please check it

Title:go-kardia Potential file inclusion (CWE-22) & Deferring unsafe Method (CWE-703)


Contact to me : 0x9a@tuta.io
Github: https://github.com/0x9a

Vendor Homepage: https://kardiachain.io

Software Link: https://github.com/kardiachain/go-kardia

Tested on: Kali

#Description
CWE-22:
Many file operations are intended to take place within a restricted directory.
By using special elements such as ".." and "/" separators, attackers can escape outside of the restricted location to access files or directories that are elsewhere on the system.
One of the most common special elements is the "../" sequence, which in most modern operating systems is interpreted as the parent directory of the current location.
This is referred to as relative path traversal.
Path traversal also covers the use of absolute pathnames such as "/usr/local/bin", which may also be useful in accessing unexpected files.
This is referred to as absolute path traversal.
In many programming languages, the injection of a null byte (the 0 or NUL) may allow an attacker to truncate a generated filename to widen the scope of attack. For example, the software may add ".txt" to any pathname, thus limiting the attacker to text files, but a null injection may effectively remove this restriction.

Efected file:(go-kardia/blob/master/kai/account/keystore.go)
Line=156 - 186

func (keyStore *KeyStore) GetKey(auth string) error {

	// check if address exists in path or not
	filename := keyStore.joinPath() // for example: /etc/passwd
	if _, err := os.Stat(filename); os.IsNotExist(err) {
		return err
	}

	fd, err := os.Open(filename) // Vulnerability Line
	if err != nil {
		return err
	}
	//155-169

#Description
CWE-703:
whenever you conjure up a value that implements the io.Closer interface, after checking for errors you immediately defer its Close() method.
You see this most often when making #HTTP requests or #opening files:

Efected file:(go-kardia/blob/master/kai/account/keystore.go)
Line=156 - 186

func (keyStore *KeyStore) GetKey(auth string) error {

	// check if address exists in path or not
	filename := keyStore.joinPath()
	if _, err := os.Stat(filename); os.IsNotExist(err) {
		return err
	}

	fd, err := os.Open(filename) // Open File
	if err != nil {
		return err
	}
	defer fd.Close() // Vulnerability Line
	key := new(KeyStoreJson)

//156 - 169

But this idiom is actually harmful for writable files because deferring a function call ignores its return value, and the Close() method can return errors.
For writable files, Go programmers should avoid the defer idiom or very infrequent, maddening bugs will occur.

The simplest way to solve this is simply not to use #defer when writing files:

fd, err := os.Open(filename) // Open File
	if err != nil {
		return err
	}
	return fd.Close() // Fixed Vulnerability
	key := new(KeyStoreJson)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant