session: detect invalid sid in Read method (gogs/gogs#5469)

go-macaron · Oct 24, 2018 · 084f1e5 · 084f1e5
1 parent b8e286a
commit 084f1e5
Show file tree

Hide file tree

Showing 3 changed files with 35 additions and 16 deletions.
diff --git a/.travis.yml b/.travis.yml
@@ -1,16 +1,11 @@
 sudo: false
 language: go
-
 go:
-  - 1.4
-  - 1.5
-  - 1.6
-  - 1.7
-  - 1.8
-  - tip
+  - 1.6.x
+  - 1.7.x
+  - 1.8.x
+  - 1.9.x
+  - 1.10.x
+  - 1.11.x
 
 script: go test -v -cover -race
-
-notifications:
-  email:
-    - u@gogs.io
diff --git a/session.go b/session.go
@@ -18,15 +18,17 @@ package session
 
 import (
 	"encoding/hex"
+	"errors"
 	"fmt"
 	"net/http"
 	"net/url"
+	"strings"
 	"time"
 
 	"gopkg.in/macaron.v1"
 )
 
-const _VERSION = "0.3.0"
+const _VERSION = "0.4.0"
 
 func Version() string {
 	return _VERSION
@@ -245,8 +247,8 @@ func NewManager(name string, opt Options) (*Manager, error) {
 	return &Manager{p, opt}, p.Init(opt.Maxlifetime, opt.ProviderConfig)
 }
 
-// sessionId generates a new session ID with rand string, unix nano time, remote addr by hash function.
-func (m *Manager) sessionId() string {
+// sessionID generates a new session ID with rand string, unix nano time, remote addr by hash function.
+func (m *Manager) sessionID() string {
 	return hex.EncodeToString(generateRandomKey(m.opt.IDLength / 2))
 }
 
@@ -258,7 +260,7 @@ func (m *Manager) Start(ctx *macaron.Context) (RawStore, error) {
 		return m.provider.Read(sid)
 	}
 
-	sid = m.sessionId()
+	sid = m.sessionID()
 	sess, err := m.provider.Read(sid)
 	if err != nil {
 		return nil, err
@@ -282,6 +284,12 @@ func (m *Manager) Start(ctx *macaron.Context) (RawStore, error) {
 
 // Read returns raw session store by session ID.
 func (m *Manager) Read(sid string) (RawStore, error) {
+	// No slashes or dots "./" should ever occur in the sid and to prevent session file forgery bug.
+	// See https://github.com/gogs/gogs/issues/5469
+	if strings.ContainsAny(sid, "./") {
+		return nil, errors.New("invalid 'sid': " + sid)
+	}
+
 	return m.provider.Read(sid)
 }
 
@@ -308,7 +316,7 @@ func (m *Manager) Destory(ctx *macaron.Context) error {
 
 // RegenerateId regenerates a session store from old session ID to new one.
 func (m *Manager) RegenerateId(ctx *macaron.Context) (sess RawStore, err error) {
-	sid := m.sessionId()
+	sid := m.sessionID()
 	oldsid := ctx.GetCookie(m.opt.CookieName)
 	sess, err = m.provider.Regenerate(oldsid, sid)
 	if err != nil {

diff --git a/session_test.go b/session_test.go
@@ -17,6 +17,7 @@ package session
 import (
 	"net/http"
 	"net/http/httptest"
+	"strings"
 	"testing"
 	"time"
 
@@ -171,6 +172,21 @@ func testProvider(opt Options) {
 		So(err, ShouldBeNil)
 		m.ServeHTTP(resp, req)
 	})
+
+	Convey("Detect invalid sid", func() {
+		m := macaron.New()
+		m.Use(Sessioner(opt))
+		m.Get("/", func(ctx *macaron.Context, sess Store) {
+			raw, err := sess.Read("../session/ad2c7e3cbecfcf486")
+			So(strings.Contains(err.Error(), "invalid 'sid': "), ShouldBeTrue)
+			So(raw, ShouldBeNil)
+		})
+
+		resp := httptest.NewRecorder()
+		req, err := http.NewRequest("GET", "/", nil)
+		So(err, ShouldBeNil)
+		m.ServeHTTP(resp, req)
+	})
 }
 
 func Test_Flash(t *testing.T) {