Skip to content
This repository has been archived by the owner on Jan 13, 2024. It is now read-only.

Possible data corruption(Plugin Failure) in x64 environment due to usage of int where intptr_t should have been used #74

Open
mahee96 opened this issue May 26, 2021 · 2 comments

Comments

@mahee96
Copy link
Collaborator

mahee96 commented May 26, 2021

Based on Scintilla the type 'intptr_t' in the Scintilla.iface file is based on the execution-image-memory-model dependent ie: it will be 32bits in x86 env whereas 64-bits in x64 env.

Hence scintilla docs suggests to use "intptr_t" type which is a pointer container that actually is size allocated based on the execution memory model for user-defined types such as Sci_Position, uptr_t, sptr_t.

If we take that into fact, then the x64 plugins using the .NET 'int'(standard 32-bit irrespective of memory model) would be losing data if an overflow occurs.

This is can be observed as below:

// interface declaration: IScintillaGateway in ScintillaGateway.cs
int GetCurrentLineNumber();

// class which implements the interface: ScintillaGateway: IScintillaGateway  in ScintillaGateway.cs
public int GetCurrentLineNumber()
        {
            return LineFromPosition(GetCurrentPos());
        }

public int GetCurrentPos()
        {
            return (int)Win32.SendMessage(scintilla, SciMsg.SCI_GETCURRENTPOS, (IntPtr) Unused, (IntPtr) Unused);
        }

public int LineFromPosition(int pos)
        {
            return (int)Win32.SendMessage(scintilla, SciMsg.SCI_LINEFROMPOSITION, (IntPtr) pos, (IntPtr) Unused);
        }

// struct definition for ScNotification in Scintilla_iface.cs
[StructLayout(LayoutKind.Sequential)]
    public struct ScNotification
    {
        public ScNotificationHeader Header;
        private IntPtr position;               /* SCN_STYLENEEDED, SCN_DOUBLECLICK, SCN_MODIFIED, SCN_MARGINCLICK, SCN_NEEDSHOWN, SCN_DWELLSTART, SCN_DWELLEND, SCN_CALLTIPCLICK, SCN_HOTSPOTCLICK, SCN_HOTSPOTDOUBLECLICK, SCN_HOTSPOTRELEASECLICK, SCN_INDICATORCLICK, SCN_INDICATORRELEASE, SCN_USERLISTSELECTION, SCN_AUTOCSELECTION */
        public int character;               /* SCN_CHARADDED, SCN_KEY, SCN_AUTOCCOMPLETE, SCN_AUTOCSELECTION, SCN_USERLISTSELECTION */
        public int Mmodifiers;              /* SCN_KEY, SCN_DOUBLECLICK, SCN_HOTSPOTCLICK, SCN_HOTSPOTDOUBLECLICK, SCN_HOTSPOTRELEASECLICK, SCN_INDICATORCLICK, SCN_INDICATORRELEASE */
        public int ModificationType;        /* SCN_MODIFIED - modification types are name "SC_MOD_*" */
        public IntPtr TextPointer;          /* SCN_MODIFIED, SCN_USERLISTSELECTION, SCN_AUTOCSELECTION, SCN_URIDROPPED */
        public int Length;                  /* SCN_MODIFIED */
        public int LinesAdded;              /* SCN_MODIFIED */
        public int Message;                 /* SCN_MACRORECORD */
        public IntPtr wParam;               /* SCN_MACRORECORD */
        public IntPtr lParam;               /* SCN_MACRORECORD */

        /// <summary>
        /// 0-based index
        /// </summary>
        public int LineNumber;           /* SCN_MODIFIED */
        public int FoldLevelNow;         /* SCN_MODIFIED */
        public int FoldLevelPrev;        /* SCN_MODIFIED */
        public int Margin;               /* SCN_MARGINCLICK */
        public int ListType;             /* SCN_USERLISTSELECTION */
        public int X;                    /* SCN_DWELLSTART, SCN_DWELLEND */
        public int Y;                    /* SCN_DWELLSTART, SCN_DWELLEND */
        public int Token;                /* SCN_MODIFIED with SC_MOD_CONTAINER */
        public int AnnotationLinesAdded; /* SC_MOD_CHANGEANNOTATION */
        public int Updated;              /* SCN_UPDATEUI */
        public int ListCompletionMethod; /* SCN_AUTOCSELECTION, SCN_AUTOCCOMPLETED, SCN_USERLISTSELECTION */

        /// <summary>
        /// SCN_STYLENEEDED, SCN_DOUBLECLICK, SCN_MODIFIED, SCN_MARGINCLICK, SCN_NEEDSHOWN, SCN_DWELLSTART, SCN_DWELLEND, SCN_CALLTIPCLICK, SCN_HOTSPOTCLICK, SCN_HOTSPOTDOUBLECLICK, SCN_HOTSPOTRELEASECLICK, SCN_INDICATORCLICK, SCN_INDICATORRELEASE, SCN_USERLISTSELECTION, SCN_AUTOCSELECTION
        /// </summary>
        public Position Position { get { return new Position(position); } }

        /// <summary>
        /// Character of the notification - eg keydown
        /// SCN_CHARADDED, SCN_KEY, SCN_AUTOCCOMPLETE, SCN_AUTOCSELECTION, SCN_USERLISTSELECTION
        /// </summary>
        public char Character { get { return (char) character; } }
    }

For reference (from) Scintilla.h:

// Definitions of common types
#ifdef _WIN64
    typedef unsigned __int64 size_t;
    typedef __int64          ptrdiff_t;
    typedef __int64          intptr_t;
#else
    typedef unsigned int     size_t;
    typedef int              ptrdiff_t;
    typedef int              intptr_t;
#endif
#ifndef _UINTPTR_T_DEFINED
    #define _UINTPTR_T_DEFINED
    #ifdef _WIN64
        typedef unsigned __int64  uintptr_t;
    #else
        typedef unsigned int uintptr_t;
    #endif
#endif

// Basic signed type used throughout interface
typedef ptrdiff_t Sci_Position;
// Define uptr_t, an unsigned integer type large enough to hold a pointer.
typedef uintptr_t uptr_t;
// Define sptr_t, a signed integer large enough to hold a pointer.
typedef intptr_t sptr_t;

struct Sci_NotifyHeader {
	/* Compatible with Windows NMHDR.
	 * hwndFrom is really an environment specific window handle or pointer
	 * but most clients of Scintilla.h do not have this type visible. */
	void *hwndFrom;
	uptr_t idFrom;
	unsigned int code;
};

struct SCNotification {
	Sci_NotifyHeader nmhdr;
	Sci_Position position;
	/* SCN_STYLENEEDED, SCN_DOUBLECLICK, SCN_MODIFIED, SCN_MARGINCLICK, */
	/* SCN_NEEDSHOWN, SCN_DWELLSTART, SCN_DWELLEND, SCN_CALLTIPCLICK, */
	/* SCN_HOTSPOTCLICK, SCN_HOTSPOTDOUBLECLICK, SCN_HOTSPOTRELEASECLICK, */
	/* SCN_INDICATORCLICK, SCN_INDICATORRELEASE, */
	/* SCN_USERLISTSELECTION, SCN_AUTOCSELECTION */

	int ch;
	/* SCN_CHARADDED, SCN_KEY, SCN_AUTOCCOMPLETED, SCN_AUTOCSELECTION, */
	/* SCN_USERLISTSELECTION */
	int modifiers;
	/* SCN_KEY, SCN_DOUBLECLICK, SCN_HOTSPOTCLICK, SCN_HOTSPOTDOUBLECLICK, */
	/* SCN_HOTSPOTRELEASECLICK, SCN_INDICATORCLICK, SCN_INDICATORRELEASE, */

	int modificationType;	/* SCN_MODIFIED */
	const char *text;
	/* SCN_MODIFIED, SCN_USERLISTSELECTION, SCN_AUTOCSELECTION, SCN_URIDROPPED */

	Sci_Position length;		/* SCN_MODIFIED */
	Sci_Position linesAdded;	/* SCN_MODIFIED */
	int message;	/* SCN_MACRORECORD */
	uptr_t wParam;	/* SCN_MACRORECORD */
	sptr_t lParam;	/* SCN_MACRORECORD */
	Sci_Position line;		/* SCN_MODIFIED */
	int foldLevelNow;	/* SCN_MODIFIED */
	int foldLevelPrev;	/* SCN_MODIFIED */
	int margin;		/* SCN_MARGINCLICK */
	int listType;	/* SCN_USERLISTSELECTION */
	int x;			/* SCN_DWELLSTART, SCN_DWELLEND */
	int y;		/* SCN_DWELLSTART, SCN_DWELLEND */
	int token;		/* SCN_MODIFIED with SC_MOD_CONTAINER */
	Sci_Position annotationLinesAdded;	/* SCN_MODIFIED with SC_MOD_CHANGEANNOTATION */
	int updated;	/* SCN_UPDATEUI */
	int listCompletionMethod;
	/* SCN_AUTOCSELECTION, SCN_AUTOCCOMPLETED, SCN_USERLISTSELECTION, */
	int characterSource;	/* SCN_CHARADDED */
};

which should have been as follows:

// interface declaration: IScintillaGateway in ScintillaGateway.cs
long GetCurrentLineNumber();

// class which implements the interface: ScintillaGateway: IScintillaGateway  in ScintillaGateway.cs
public long GetCurrentLineNumber()
        {
            return LineFromPosition(GetCurrentPos());
        }

public long GetCurrentPos()
        {
            return (long)Win32.SendMessage(scintilla, SciMsg.SCI_GETCURRENTPOS, Unused, Unused);
        }

public long LineFromPosition(long pos)
        {
            return (long)Win32.SendMessage(scintilla, SciMsg.SCI_LINEFROMPOSITION, (IntPtr) pos, Unused);
        }

// struct definition for ScNotification in Scintilla_iface.cs
[StructLayout(LayoutKind.Sequential)]
    public struct ScNotification
    {
        public ScNotificationHeader Header;
        public IntPtr position;              
        public int character;               
        public int Mmodifiers;              /* SCN_KEY, SCN_DOUBLECLICK, SCN_HOTSPOTCLICK, SCN_HOTSPOTDOUBLECLICK, SCN_HOTSPOTRELEASECLICK, SCN_INDICATORCLICK, SCN_INDICATORRELEASE */
        public int ModificationType;        /* SCN_MODIFIED - modification types are name "SC_MOD_*" */
        public IntPtr TextPointer;          /* SCN_MODIFIED, SCN_USERLISTSELECTION, SCN_AUTOCSELECTION, SCN_URIDROPPED */
        public IntPtr Length;                  /* SCN_MODIFIED */
        public IntPtr LinesAdded;              /* SCN_MODIFIED */
        public int Message;                 /* SCN_MACRORECORD */
        public IntPtr wParam;               /* SCN_MACRORECORD */
        public IntPtr lParam;               /* SCN_MACRORECORD */

        public IntPtr Line;           /* SCN_MODIFIED */
        public int FoldLevelNow;         /* SCN_MODIFIED */
        public int FoldLevelPrev;        /* SCN_MODIFIED */
        public int Margin;               /* SCN_MARGINCLICK */
        public int ListType;             /* SCN_USERLISTSELECTION */
        public int X;                    /* SCN_DWELLSTART, SCN_DWELLEND */
        public int Y;                    /* SCN_DWELLSTART, SCN_DWELLEND */
        public int Token;                /* SCN_MODIFIED with SC_MOD_CONTAINER */
        public IntPtr AnnotationLinesAdded; /* SC_MOD_CHANGEANNOTATION */
        public int Updated;              /* SCN_UPDATEUI */
        public int ListCompletionMethod; /* SCN_AUTOCSELECTION, SCN_AUTOCCOMPLETED, SCN_USERLISTSELECTION */
        public int characterSource;	/* SCN_CHARADDED */
        
        /// <summary>
        /// SCN_STYLENEEDED, SCN_DOUBLECLICK, SCN_MODIFIED, SCN_MARGINCLICK, SCN_NEEDSHOWN, SCN_DWELLSTART, SCN_DWELLEND, SCN_CALLTIPCLICK, SCN_HOTSPOTCLICK, SCN_HOTSPOTDOUBLECLICK, SCN_HOTSPOTRELEASECLICK, SCN_INDICATORCLICK, SCN_INDICATORRELEASE, SCN_USERLISTSELECTION, SCN_AUTOCSELECTION
        /// </summary>
        public Position Position { get { return new Position(position); } }

        /// <summary>
        /// Character of the notification - eg keydown
        /// SCN_CHARADDED, SCN_KEY, SCN_AUTOCCOMPLETE, SCN_AUTOCSELECTION, SCN_USERLISTSELECTION
        /// </summary>
        public char Character { get { return (char)character; } }
    }

@kbilsted look at the updated version, the long maintains the data to be 64-bit wide so that the data whatsoever it may be, 32bits or 64-bits contained in the IntPtr being returned from the SendMessage Win32k Api call to C++ application.

This should also take care of the arithmetic/logical operation issues which would ensue if using the IntPtr in function/method signatures instead of long.

The "long" would waste 32-bits of storage for each allocation because in 32-bit environment the returned data from IntPtr would be of size "int", but will save us from hassle of the above mentioned issues with operators with a slight cost to storage.

Note: However the underlying mapping "structure of the ScNotification struct, needs to use IntPtr since, the data alignment should match the data coming from C++ code.

@Fruchtzwerg94
Copy link
Contributor

Is there any update on this issue?

I've stumbled over the same behaviour at this minimal sample:
TextRange textRange = new TextRange(0, -1, 1000)
which works for x86 but crashes on for x64.

After modifying to IntPtr it runs for both architectures:
TextRange textRange = new TextRange(new IntPtr(0), new IntPtr(-1), 1000)

https://github.com/kbilsted/NotepadPlusPlusPluginPack.Net/blob/master/Visual%20Studio%20Project%20Template%20C%23/PluginInfrastructure/GatewayDomain.cs#L191
https://github.com/kbilsted/NotepadPlusPlusPluginPack.Net/blob/master/Visual%20Studio%20Project%20Template%20C%23/PluginInfrastructure/Scintilla_iface.cs#L3233

@Fruchtzwerg94
Copy link
Contributor

See #91

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants