From e3bbc16a8c186467458fd8713b6cac95fcdb35f5 Mon Sep 17 00:00:00 2001 From: sourque Date: Thu, 27 Oct 2022 02:17:28 -0700 Subject: [PATCH] Fix out-of-bounds for UserInGroup on Windows --- README.md | 8 ++++---- checks.go | 4 +++- checks_linux.go | 2 +- checks_windows.go | 10 ++++++++-- configs.go | 9 +++++---- crypto.go | 6 +++--- docs/config.md | 4 ++-- score.go | 8 ++++---- utility.go | 2 +- 9 files changed, 31 insertions(+), 22 deletions(-) diff --git a/README.md b/README.md index 3de6a4d7..b93f227d 100644 --- a/README.md +++ b/README.md @@ -146,14 +146,14 @@ message = "Malicious user 'user' can't read /etc/shadow" type = "CommandNot" cmd = "sudo -u user cat /etc/shadow" - [[check.pass]] # "pass" conditions are logically AND with other pass - type = "FileExists" # conditions. This means they all must pass for a check - path = "/etc/shadow" # to be considered successful. + [[check.pass]] # "pass" conditions are logically AND with other pass + type = "FileExists" # conditions. This means they all must pass for a check + path = "/etc/shadow" # to be considered successful. [[check.passoverride]] # If you a check to succeed if just one condition type = "UserExistsNot" # passes, regardless of other pass checks, use user = "user" # an override pass (passoverride). This is a logical OR. - # passoverride is overridden by fail conditions. + # passoverride is overridden by fail conditions. [[check.fail]] # If any fail conditions succeed, the entire check will fail. type = "FileExistsNot" diff --git a/checks.go b/checks.go index 1c72eab9..593d94f7 100644 --- a/checks.go +++ b/checks.go @@ -38,6 +38,8 @@ type cond struct { After string } +// requireArgs is a convenience function that prints a warning if any required +// parameters for a given condition are not provided. func (c cond) requireArgs(args ...interface{}) { // Don't process internal calls -- assume the developers know what they're // doing. This also prevents extra errors being printed when they don't pass @@ -105,7 +107,7 @@ func runCheck(cond cond) bool { // Ensure that condition type is a valid length if len(cond.Type) <= len(not) { - fail(`Condition type "` + cond.Type + `" is not long enough to be valid`) + fail(`Condition type "` + cond.Type + `" is not long enough to be valid. Do you have a "type = 'CheckTypeHere'" for all check conditions?`) return false } diff --git a/checks_linux.go b/checks_linux.go index 582e09f6..f28abcf6 100644 --- a/checks_linux.go +++ b/checks_linux.go @@ -145,7 +145,7 @@ func (c cond) ProgramInstalled() (bool, error) { func (c cond) ProgramVersion() (bool, error) { c.requireArgs("Name", "Value") return cond{ - Cmd: `dpkg -s ` + c.Name + ` | grep Version | cut -d" " -f2`, + Cmd: `dpkg -s ` + c.Name + ` | grep Version | cut -d" " -f2`, Value: c.Value, }.CommandOutput() } diff --git a/checks_windows.go b/checks_windows.go index 1da09a17..a9ef5ba8 100644 --- a/checks_windows.go +++ b/checks_windows.go @@ -374,10 +374,16 @@ func (c cond) UserInGroup() (bool, error) { return false, nil } for _, user := range users { - justName := strings.Split(user.Name, `\`)[1] - if c.User == user.Name || c.User == justName { + if c.User == user.Name { return true, nil } + // If username contains a backslash (for hostname or domain), compare + // against only the second part as well + if splitName := strings.Split(user.Name, `\`); len(splitName) > 1 { + if c.User == splitName[1] { + return true, nil + } + } } return false, nil } diff --git a/configs.go b/configs.go index 6759f97c..66a3d954 100644 --- a/configs.go +++ b/configs.go @@ -24,7 +24,7 @@ func parseConfig(configContent string) { } if verboseEnabled { for _, undecoded := range md.Undecoded() { - warn("Undecoded scoring key \"" + undecoded.String() + "\" will not be used") + warn("Undecoded scoring configuration key \"" + undecoded.String() + "\" will not be used.") } } @@ -51,14 +51,15 @@ func parseConfig(configContent string) { info(" version = '" + version + "'") } + // Print warnings for impossible checks and undefined check types. for i, check := range conf.Check { allConditions := append(append(append([]cond{}, check.Pass[:]...), check.Fail[:]...), check.PassOverride[:]...) if len(allConditions) == 0 { - warn("Check " + fmt.Sprintf("%d", i+1) + " does not define any possible ways to pass") + warn("Check " + fmt.Sprintf("%d", i+1) + " does not define any possible ways to pass!") } for j, cond := range allConditions { if cond.Type == "" { - warn("Check " + fmt.Sprintf("%d condition %d", i+1, j+1) + " has an empty type and will crash at runtime") + warn("Check " + fmt.Sprintf("%d condition %d", i+1, j+1) + " does not have a check type!") } } } @@ -187,7 +188,7 @@ func obfuscateConfig() { } // obfuscateCond is a convenience function to obfuscate all string fields of a -// struct using reflection. ONLY use it on a struct of strings. +// struct using reflection. It assumes all struct fields are strings. func obfuscateCond(c *cond) error { s := reflect.ValueOf(c).Elem() for i := 0; i < s.NumField(); i++ { diff --git a/crypto.go b/crypto.go index 832dea19..0eee5ae7 100644 --- a/crypto.go +++ b/crypto.go @@ -12,7 +12,7 @@ // // If you compile the source code yourself, using the Makefile, random strings // will be generated for you. This means that the pre-compiled release will no -// longer work for decrypting your configs-- which is ideal. +// longer work for decrypting your configs, which is good. package main @@ -260,14 +260,14 @@ func decryptString(password, ciphertext string) string { // Create the AES-GCM cipher with the generated block. aesgcm, err := cipher.NewGCM(block) if err != nil { - fail(err.Error()) + fail("Error creating AES cipher (please tell the developers):", err.Error()) return "" } // Decrypt (and check validity, since it's GCM) of ciphertext. plainText, err := aesgcm.Open(nil, iv, []byte(ciphertext), nil) if err != nil { - fail(err.Error()) + fail("Error decrypting (are you using the correct aeacus/phocus? you may need to re-encrypt your config):", err.Error()) return "" } diff --git a/docs/config.md b/docs/config.md index 686cb65a..bf0281a9 100644 --- a/docs/config.md +++ b/docs/config.md @@ -56,10 +56,10 @@ local = true enddate = "2004/06/05 13:09:00 PDT" ``` -**shell**: Determines if remote shell functionality is enabled. This is disabled by default. If enabled, competition organizers can interact with images from the scoring endpoint +**shell**: (Warning: the canonical remote endpoint (sarpedon) does not support this feature). Determines if remote shell functionality is enabled. This is disabled by default. If enabled, competition organizers can interact with images from the scoring endpoint ``` -shell = true +shell = false ``` **version**: Version of aeacus that the configuration was made for. Used for compatibility checks, the engine will throw a warning if the binary version does not match the version specified in this field. You should set this to the version of aeacus you are using. diff --git a/score.go b/score.go index 0fd7c2b5..d4f81c7c 100644 --- a/score.go +++ b/score.go @@ -67,10 +67,11 @@ type statusRes struct { Status string `json:"status"` } -// ReadScoringData is a convenience function around readData and decodeString, +// readScoringData is a convenience function around readData and decodeString, // which parses the encrypted scoring configuration file. func readScoringData() error { info("Decrypting data from " + dirPath + scoringData + "...") + // Read in the encrypted configuration file dataFile, err := readFile(dirPath + scoringData) if err != nil { @@ -78,10 +79,8 @@ func readScoringData() error { } else if dataFile == "" { return errors.New("Scoring data is empty!") } + decryptedData, err := decryptConfig(dataFile) - if err != nil { - return err - } if err != nil { fail("Error reading in scoring data: " + err.Error()) return err @@ -91,6 +90,7 @@ func readScoringData() error { } else { info("Data decryption successful!") } + parseConfig(decryptedData) return nil } diff --git a/utility.go b/utility.go index 69d65059..a22298d6 100644 --- a/utility.go +++ b/utility.go @@ -10,7 +10,7 @@ import ( ) const ( - version = "2.0.3" + version = "2.0.4" ) var (