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

orm/model/ormdb: (moduleDB).DefaultJSON writes table JSON non-deterministically due to map iteration #12431

Closed
2 tasks done
odeke-em opened this issue Jul 3, 2022 · 0 comments · Fixed by #12751
Closed
2 tasks done

Comments

@odeke-em
Copy link
Collaborator

odeke-em commented Jul 3, 2022

Summary of Bug

If we audit the code in

for name, table := range m.tablesByName {
w, err := target.OpenWriter(name)
if err != nil {
return err
}
_, err = w.Write(table.DefaultJSON())
if err != nil {
return err
}
err = w.Close()
if err != nil {
return err
}
}
we can see that the various tables' JSON is written to dependent only on the map's iteration order

Version

77cf430

Fix

diff --git a/orm/model/ormdb/json.go b/orm/model/ormdb/json.go
index 3a64ae52f..79a668ccd 100644
--- a/orm/model/ormdb/json.go
+++ b/orm/model/ormdb/json.go
@@ -15,7 +15,17 @@ import (
 )
 
 func (m moduleDB) DefaultJSON(target ormjson.WriteTarget) error {
-	for name, table := range m.tablesByName {
+	tableNames := make([]protoreflect.FullName, 0, len(m.tablesByName))
+	for name := range m.tablesByName {
+		tableNames = append(tableNames, name)
+	}
+	sort.Slice(tableNames, func(i, j int) bool {
+		ti, tj := tableNames[i], tableNames[j]
+		return ti.Name() < tj.Name()
+	})
+
+	for _, name := range tableNames {
+		table := m.tablesByName[name]
 		w, err := target.OpenWriter(name)
 		if err != nil {
 			return err

For Admin Use

  • Not duplicate issue
  • Appropriate labels applied
odeke-em added a commit that referenced this issue Jul 27, 2022
Reported by informalsystems/gosec's map iteration pass, this
change fixes non-determinism from iterating with a map, whose
definition in the Go spec guarantees that the order of keys
and values presented during iteration will be randomized.

Fixes #12428
Fixes #12430
Fixes #12431
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

Successfully merging a pull request may close this issue.

1 participant