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

panic: runtime error: index out of range [0] with length 0 #3036

Closed
Ali-Razmjoo opened this issue Dec 13, 2022 · 11 comments · Fixed by #3041
Closed

panic: runtime error: index out of range [0] with length 0 #3036

Ali-Razmjoo opened this issue Dec 13, 2022 · 11 comments · Fixed by #3041
Assignees
Labels
Status: Completed Nothing further to be done with this issue. Awaiting to be closed. Type: Bug Inconsistencies or issues which will cause an issue or problem for users or implementors.
Milestone

Comments

@Ali-Razmjoo
Copy link

Ali-Razmjoo commented Dec 13, 2022

Hi,

I wanted to report an issue; I am using the latest version of nuclei (2.8.3) compiled for windows 64bit. I have a list of hosts with the following format https://ip:port and after a while, I get the below error message on one of the hosts (not sure which one).

panic: runtime error: index out of range [0] with length 0

goroutine 14686 [running]:
github.com/zmap/zcrypto/verifier.CheckOCSP({0xxxxxxx, 0xxxxxxx}, 0xxxxxxx, 0xxxxxxx?)
        github.com/zmap/zcrypto@v0.0.0-xxxxxx-xxxxxx/verifier/revocation.go:52 +0x57c
github.com/projectdiscovery/tlsx/pkg/tlsx/clients.IsZTLSRevoked(0xxxxxxx)
        github.com/projectdiscovery/tlsx@v1.0.0/pkg/tlsx/clients/clients.go:327 +0xxxxxxx
github.com/projectdiscovery/tlsx/pkg/tlsx/clients.IsTLSRevoked(0xxxxxxx?)
        github.com/projectdiscovery/tlsx@v1.0.0/pkg/tlsx/clients/clients.go:321 +0xxxxxxx
github.com/projectdiscovery/tlsx/pkg/tlsx/tls.(*Client).convertCertificateToResponse(0xxxxxxx, {0xxxxxxx, 0xe}, 0xxxxxxx)
        github.com/projectdiscovery/tlsx@v1.0.0/pkg/tlsx/tls/tls.go:205 +0xxxxxxx
github.com/projectdiscovery/tlsx/pkg/tlsx/tls.(*Client).ConnectWithOptions(0xxxxxxx, {0xxxxxxx, 0xe}, {0xxxxxxx?, 0xe?}, {0xxxxxxx, 0x3}, {{0x0, 0x0}, {0x0, ...}, ...})
        github.com/projectdiscovery/tlsx@v1.0.0/pkg/tlsx/tls/tls.go:185 +0x8aa
github.com/projectdiscovery/tlsx/pkg/tlsx/auto.(*Client).ConnectWithOptions(0xxxxxxx, {0xxxxxxx, 0xe}, {0xxxxxxx, 0xe}, {0xxxxxxx, 0x3}, {{0x0, 0x0}, {0x0, ...}, ...})
        github.com/projectdiscovery/tlsx@v1.0.0/pkg/tlsx/auto/auto.go:38 +0xc5
github.com/projectdiscovery/tlsx/pkg/tlsx.(*Service).ConnectWithOptions(0xxxxxxx, {0xxxxxxx, 0xe}, {0xxxxxxx, 0xe}, {0xxxxxxx, 0x3}, {{0x0, 0x0}, {0x0, ...}, ...})
        github.com/projectdiscovery/tlsx@v1.0.0/pkg/tlsx/tlsx.go:66 +0x116
github.com/projectdiscovery/tlsx/pkg/tlsx.(*Service).Connect(...)
        github.com/projectdiscovery/tlsx@v1.0.0/pkg/tlsx/tlsx.go:57
github.com/projectdiscovery/nuclei/v2/pkg/protocols/ssl.(*Request).ExecuteWithResults(0xxxxxxx, 0xxxxxxx, 0xxxxxxx?, 0xxxxxxx?, 0xxxxxxx)
        github.com/projectdiscovery/nuclei/v2/pkg/protocols/ssl/ssl.go:184 +0xxxxxxx
github.com/projectdiscovery/nuclei/v2/pkg/protocols/common/executer.(*Executer).Execute(0xxxxxxx, 0xxxxxxx)
        github.com/projectdiscovery/nuclei/v2/pkg/protocols/common/executer/executer.go:80 +0xxxxxxx
github.com/projectdiscovery/nuclei/v2/pkg/core.(*Engine).executeModelWithInput.func2.1(0xxxxxxx?, 0xc0?, 0xxxxxxx)
        github.com/projectdiscovery/nuclei/v2/pkg/core/execute.go:146 +0xxxxxxx
created by github.com/projectdiscovery/nuclei/v2/pkg/core.(*Engine).executeModelWithInput.func2
        github.com/projectdiscovery/nuclei/v2/pkg/core/execute.go:131 +0xxxxxxx

I am using this command line to perform the scan:

nuclei -l x.txt -o info.txt -nc -stats -c 100 -rl 300 -t C:\Users\x\nuclei-templates\ssl\ssl-dns-names.yaml

Unfortunately, I can't see what is in response to share it here.

Bests, Ali.

@Ali-Razmjoo Ali-Razmjoo added the Type: Bug Inconsistencies or issues which will cause an issue or problem for users or implementors. label Dec 13, 2022
@tarunKoyalwar tarunKoyalwar self-assigned this Dec 14, 2022
@vzamanillo
Copy link
Contributor

Looks like some intermediate root certificate has a missing / empty OSCP URI location.

@jimen0
Copy link
Contributor

jimen0 commented Dec 14, 2022

This comes from https://github.com/zmap/zcrypto/blob/557f3e4940be18b8c2d631190644e15d99441b29/verifier/revocation.go#L52

https://github.com/zmap/zcrypto/blob/557f3e4940be18b8c2d631190644e15d99441b29/x509/x509.go#L2100-L2112

It will need to be fixed upstream on zcrypto and then the dependency can be updated on the tlsx side. cc @forgedhallpass @Ice3man543 in case you want to open the upstream issue.

@tarunKoyalwar
Copy link
Member

@jimen0 @vzamanillo @Ali-Razmjoo a similar issue was recently fixed at projectdiscovery/tlsx#127 . extra error handling should solve this

@jimen0
Copy link
Contributor

jimen0 commented Dec 14, 2022

The certificate is obtained by tlsx here: https://sourcegraph.com/github.com/projectdiscovery/tlsx@v1.0.0/-/blob/pkg/tlsx/tls/tls.go?L165-172

Is a certificate without OCSPServer values valid, @tarunKoyalwar? If not, then the correct fix here is to make sure zcryto returns an error on the validation call made here: https://github.com/zmap/zcrypto/blob/557f3e4940be18b8c2d631190644e15d99441b29/x509/x509.go#L1561

If these are valid certs, then I agree with tlsx doing an extra check prior to the revokation check being the correct fix.

Edit: per https://www.rfc-editor.org/rfc/rfc5280#section-5.2.7, when present, at least 1 OCSP sever must be listed.

When present in a CRL, this extension MUST include at least one
AccessDescription specifying id-ad-caIssuers as the accessMethod.

@tarunKoyalwar
Copy link
Member

@jimen0 , thanks for your response and pointing out error but as you already know browser have stopped using OCSP to validate (disabled by default ) . and use CRL Set/List to check if ceritificate is revoked . and these CRLsets are maintained by browsers and regularly updated with each update . https://www.ssl.com/blogs/how-do-browsers-handle-revoked-ssl-tls-certificates/

If you look at tlsx code

OCSPisRevoked, _, OCSPerr := zverifier.CheckOCSP(context.TODO(), cert, nil)
	if len(cert.CRLDistributionPoints) != 0 {
		CRLisRevoked, _, CRLerr := zverifier.CheckCRL(context.TODO(), cert, nil)

		if CRLerr == nil {
			if OCSPerr == nil {
				return OCSPisRevoked || CRLisRevoked
			} else {
				return CRLisRevoked
			}
		}
	}

tlsx does both OCSP and CRL checking . with peculiarity to always parse as much as possible . while browsers only use (CRLSet) hence we will be using Soft-fail statergy to allow missing values as valid since they are already validated with CRLs

I hope this resolves your issue

@tarunKoyalwar
Copy link
Member

@Ali-Razmjoo , this will be resolved by error handling in tlsx at projectdiscovery/tlsx#137 . with a upstream patch later . you can expect this is to resolved in next bug fix(minor) release

@vzamanillo
Copy link
Contributor

The applied fix on projectdiscovery/tlsx#138 it's fine but regardless of this I think zcrypto should return a certificate parsing error as @jimen0 said before. We should fill an issue.

@tarunKoyalwar
Copy link
Member

yup I will create upstream issue/PR for this

@ehsandeep ehsandeep added this to the nuclei v2.8.4 milestone Dec 14, 2022
@tarunKoyalwar
Copy link
Member

@Ali-Razmjoo , Fix is available at issue-3036-tlsx-panic branch can you verify it just in case

Run following commands

git clone https://github.com/projectdiscovery/nuclei.git
git checkout issue-3036-tlsx-panic && cd v2/cmd/nuclei 
go build . 
./nuclei -l x.txt -o info.txt -nc -stats -c 100 -rl 300 -t C:\Users\x\nuclei-templates\ssl\ssl-dns-names.yaml

you can install using go install .

#Note:
you can directly install from that specific branch but sometimes it fails .

@ehsandeep ehsandeep linked a pull request Dec 14, 2022 that will close this issue
4 tasks
@Ali-Razmjoo
Copy link
Author

Hi,

Thanks for the help; the patch you provided fixed the issue.

Bests, Ali.

ehsandeep added a commit that referenced this issue Dec 15, 2022
Co-authored-by: Tarun Koyalwar <tarun@projectdiscovery.io>
@ehsandeep ehsandeep added the Status: Completed Nothing further to be done with this issue. Awaiting to be closed. label Dec 15, 2022
@tarunKoyalwar tarunKoyalwar reopened this Dec 19, 2022
@tarunKoyalwar
Copy link
Member

@Ali-Razmjoo closing issue is handled by internal team . Status: Completed means issue is resolved

@ehsandeep ehsandeep mentioned this issue Jan 2, 2023
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status: Completed Nothing further to be done with this issue. Awaiting to be closed. Type: Bug Inconsistencies or issues which will cause an issue or problem for users or implementors.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants