Skip to content

Commit

Permalink
Fix +nnn bug in FromString
Browse files Browse the repository at this point in the history
While doing experiments with the dev.fuzz branch of the Go tool chain,
I've found a bug in FromString() within 1 second of fuzz time:

Because I don't use a regexp in the parseIdentifier, but instead simply
use strconv.Atoi, it is possible to sneak in a sign symbol.  `-` is
detected, but + is not.  There is not much harm, as an AIRAC identifier
like "+911" is interpreted as "0911", but technically it is not a valid
identifier and should be rejected.  Also, although a `-` sign correctly
yields errors, it is not clear in the code, why.

This change rejects + and - signs explicitly.
  • Loading branch information
jwkohnen committed Apr 28, 2021
1 parent 0f11ff6 commit 1563baf
Show file tree
Hide file tree
Showing 2 changed files with 8 additions and 0 deletions.
5 changes: 5 additions & 0 deletions airac.go
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,11 @@ func parseIdentifier(yyoo string) (year, ordinal int, err error) {
if len(yyoo) != 4 {
return 0, 0, fmt.Errorf("illegal AIRAC id %q", yyoo)
}

if sign := yyoo[0]; sign == '+' || sign == '-' {
return 0, 0, fmt.Errorf("illegal AIRAC id %q", yyoo)
}

yyooInt, err := strconv.Atoi(yyoo)
if err != nil {
return 0, 0, fmt.Errorf("illegal AIRAC id %q", yyoo)
Expand Down
3 changes: 3 additions & 0 deletions airac_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -235,6 +235,9 @@ func TestFromString(t *testing.T) {
{"a", "", 0, 0, false},
{"aa", "", 0, 0, false},
{"", "", 0, 0, false},

// found by fuzzer (case testdata/corpus/FuzzFromString/44ea5456f4caf7ee4b0cb896e30cb4f52cd0e65cd3c1843393c5a535f97c7a6e)
{"+911", "", 0, 0, false},
}

for i, tt := range testt {
Expand Down

0 comments on commit 1563baf

Please sign in to comment.